[libcamera-devel] [PATCH v3 13/33] cam: options: Avoid copies of OptionvValue and KeyValueParser::Options
Jacopo Mondi
jacopo at jmondi.org
Thu Jul 22 14:54:48 CEST 2021
Hi Laurent,
On Fri, Jul 16, 2021 at 12:14:39AM +0300, Laurent Pinchart wrote:
> The OptionValue toKeyValues() and toArray() functions return a copy of
> the values. This is unnecessary, and can cause use-after-free issues
> when taking references to the return values. Return references instead
> to optimize the implementation and avoid issues.
It might be good to briefly mention that error types are now rising an
assertion.
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
Thanks
j
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> Changes since v1:
>
> - assert() on undefined behaviour
> ---
> src/cam/options.cpp | 24 ++++++++++++------------
> src/cam/options.h | 4 ++--
> 2 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/src/cam/options.cpp b/src/cam/options.cpp
> index 481ac189f115..6e0d802cb986 100644
> --- a/src/cam/options.cpp
> +++ b/src/cam/options.cpp
> @@ -638,27 +638,27 @@ std::string OptionValue::toString() const
>
> /**
> * \brief Retrieve the value as a key-value list
> - * \return The option value as a KeyValueParser::Options, or an empty list if
> - * the value type isn't ValueType::ValueKeyValue
> + *
> + * The behaviour is undefined if the value type isn't ValueType::ValueKeyValue.
> + *
> + * \return The option value as a KeyValueParser::Options
> */
> -KeyValueParser::Options OptionValue::toKeyValues() const
> +const KeyValueParser::Options &OptionValue::toKeyValues() const
> {
> - if (type_ != ValueKeyValue)
> - return KeyValueParser::Options();
> -
> + assert(type_ == ValueKeyValue);
> return keyValues_;
> }
>
> /**
> * \brief Retrieve the value as an array
> - * \return The option value as a std::vector of OptionValue, or an empty vector
> - * if the value type isn't ValueType::ValueArray
> + *
> + * The behaviour is undefined if the value type isn't ValueType::ValueArray.
> + *
> + * \return The option value as a std::vector of OptionValue
> */
> -std::vector<OptionValue> OptionValue::toArray() const
> +const std::vector<OptionValue> &OptionValue::toArray() const
> {
> - if (type_ != ValueArray)
> - return std::vector<OptionValue>{};
> -
> + assert(type_ == ValueArray);
> return array_;
> }
>
> diff --git a/src/cam/options.h b/src/cam/options.h
> index 83c409ae4d28..0047b4f220ed 100644
> --- a/src/cam/options.h
> +++ b/src/cam/options.h
> @@ -142,8 +142,8 @@ public:
>
> int toInteger() const;
> std::string toString() const;
> - KeyValueParser::Options toKeyValues() const;
> - std::vector<OptionValue> toArray() const;
> + const KeyValueParser::Options &toKeyValues() const;
> + const std::vector<OptionValue> &toArray() const;
>
> const OptionsParser::Options &children() const;
>
> --
> Regards,
>
> Laurent Pinchart
>
More information about the libcamera-devel
mailing list