[libcamera-devel,v2,06/24] libcamera: controls: Add move constructor to ControlInfoMap

Message ID 20191108205409.18845-7-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • Control serialization and IPA C API
Related show

Commit Message

Laurent Pinchart Nov. 8, 2019, 8:53 p.m. UTC
The ControlInfoMap class has a move assignment operator from a plain
map, but on corresponding move constructor. Add one.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/controls.h |  1 +
 src/libcamera/controls.cpp   | 13 +++++++++++++
 2 files changed, 14 insertions(+)

Comments

Jacopo Mondi Nov. 15, 2019, 4:19 p.m. UTC | #1
Hi Laurent,

On Fri, Nov 08, 2019 at 10:53:51PM +0200, Laurent Pinchart wrote:
> The ControlInfoMap class has a move assignment operator from a plain
> map, but on corresponding move constructor. Add one.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  include/libcamera/controls.h |  1 +
>  src/libcamera/controls.cpp   | 13 +++++++++++++
>  2 files changed, 14 insertions(+)
>
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index f9979a466eaa..548c06c65bb6 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -146,6 +146,7 @@ public:
>  	ControlInfoMap() = default;
>  	ControlInfoMap(const ControlInfoMap &other) = default;
>  	ControlInfoMap(std::initializer_list<Map::value_type> init);
> +	ControlInfoMap(Map &&info);
>
>  	ControlInfoMap &operator=(const ControlInfoMap &other) = default;
>  	ControlInfoMap &operator=(std::initializer_list<Map::value_type> init);
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 021d5f0990e0..ae6ca2a7cf7e 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -445,6 +445,19 @@ ControlInfoMap::ControlInfoMap(std::initializer_list<Map::value_type> init)
>  	generateIdmap();
>  }
>
> +/**
> + * \brief Construct a ControlInfoMap from a plain map
> + * \param[in] info The control info plain map
> + *
> + * Construct a new ControlInfoMap and populate its contents with those of
> + * \a info using move  semantics. Upon return the \a info map will be empty.
                         ^ double space

> + */
> +ControlInfoMap::ControlInfoMap(Map &&info)
> +	: Map(std::move(info))
> +{
> +	generateIdmap();
> +}
> +

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
   j

>  /**
>   * \fn ControlInfoMap &ControlInfoMap::operator=(const ControlInfoMap &other)
>   * \brief Copy assignment operator, replace the contents with a copy of \a other
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Niklas Söderlund Nov. 18, 2019, 5:09 p.m. UTC | #2
Hi Laurent,

Thanks for your work.

On 2019-11-08 22:53:51 +0200, Laurent Pinchart wrote:
> The ControlInfoMap class has a move assignment operator from a plain
> map, but on corresponding move constructor. Add one.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

With the double space pointed out by Jacopo,

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  include/libcamera/controls.h |  1 +
>  src/libcamera/controls.cpp   | 13 +++++++++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index f9979a466eaa..548c06c65bb6 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -146,6 +146,7 @@ public:
>  	ControlInfoMap() = default;
>  	ControlInfoMap(const ControlInfoMap &other) = default;
>  	ControlInfoMap(std::initializer_list<Map::value_type> init);
> +	ControlInfoMap(Map &&info);
>  
>  	ControlInfoMap &operator=(const ControlInfoMap &other) = default;
>  	ControlInfoMap &operator=(std::initializer_list<Map::value_type> init);
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 021d5f0990e0..ae6ca2a7cf7e 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -445,6 +445,19 @@ ControlInfoMap::ControlInfoMap(std::initializer_list<Map::value_type> init)
>  	generateIdmap();
>  }
>  
> +/**
> + * \brief Construct a ControlInfoMap from a plain map
> + * \param[in] info The control info plain map
> + *
> + * Construct a new ControlInfoMap and populate its contents with those of
> + * \a info using move  semantics. Upon return the \a info map will be empty.
> + */
> +ControlInfoMap::ControlInfoMap(Map &&info)
> +	: Map(std::move(info))
> +{
> +	generateIdmap();
> +}
> +
>  /**
>   * \fn ControlInfoMap &ControlInfoMap::operator=(const ControlInfoMap &other)
>   * \brief Copy assignment operator, replace the contents with a copy of \a other
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Kieran Bingham Nov. 19, 2019, 3:58 p.m. UTC | #3
Hi Laurent,

On 08/11/2019 20:53, Laurent Pinchart wrote:
> The ControlInfoMap class has a move assignment operator from a plain
> map, but on corresponding move constructor. Add one.

That sentence doesn't seem to quite make sense. Could you try to
refactor it a little please?

In fact, I think it's just an 's/on/no/' :-D


> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Otherwise seems fine, with Jacopo's note.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  include/libcamera/controls.h |  1 +
>  src/libcamera/controls.cpp   | 13 +++++++++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index f9979a466eaa..548c06c65bb6 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -146,6 +146,7 @@ public:
>  	ControlInfoMap() = default;
>  	ControlInfoMap(const ControlInfoMap &other) = default;
>  	ControlInfoMap(std::initializer_list<Map::value_type> init);
> +	ControlInfoMap(Map &&info);
>  
>  	ControlInfoMap &operator=(const ControlInfoMap &other) = default;
>  	ControlInfoMap &operator=(std::initializer_list<Map::value_type> init);
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 021d5f0990e0..ae6ca2a7cf7e 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -445,6 +445,19 @@ ControlInfoMap::ControlInfoMap(std::initializer_list<Map::value_type> init)
>  	generateIdmap();
>  }
>  
> +/**
> + * \brief Construct a ControlInfoMap from a plain map
> + * \param[in] info The control info plain map
> + *
> + * Construct a new ControlInfoMap and populate its contents with those of
> + * \a info using move  semantics. Upon return the \a info map will be empty.
> + */
> +ControlInfoMap::ControlInfoMap(Map &&info)
> +	: Map(std::move(info))
> +{
> +	generateIdmap();
> +}
> +
>  /**
>   * \fn ControlInfoMap &ControlInfoMap::operator=(const ControlInfoMap &other)
>   * \brief Copy assignment operator, replace the contents with a copy of \a other
>

Patch

diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
index f9979a466eaa..548c06c65bb6 100644
--- a/include/libcamera/controls.h
+++ b/include/libcamera/controls.h
@@ -146,6 +146,7 @@  public:
 	ControlInfoMap() = default;
 	ControlInfoMap(const ControlInfoMap &other) = default;
 	ControlInfoMap(std::initializer_list<Map::value_type> init);
+	ControlInfoMap(Map &&info);
 
 	ControlInfoMap &operator=(const ControlInfoMap &other) = default;
 	ControlInfoMap &operator=(std::initializer_list<Map::value_type> init);
diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
index 021d5f0990e0..ae6ca2a7cf7e 100644
--- a/src/libcamera/controls.cpp
+++ b/src/libcamera/controls.cpp
@@ -445,6 +445,19 @@  ControlInfoMap::ControlInfoMap(std::initializer_list<Map::value_type> init)
 	generateIdmap();
 }
 
+/**
+ * \brief Construct a ControlInfoMap from a plain map
+ * \param[in] info The control info plain map
+ *
+ * Construct a new ControlInfoMap and populate its contents with those of
+ * \a info using move  semantics. Upon return the \a info map will be empty.
+ */
+ControlInfoMap::ControlInfoMap(Map &&info)
+	: Map(std::move(info))
+{
+	generateIdmap();
+}
+
 /**
  * \fn ControlInfoMap &ControlInfoMap::operator=(const ControlInfoMap &other)
  * \brief Copy assignment operator, replace the contents with a copy of \a other