[v1] libcamera: base: object: Forward arguments when invoking

23068 diff mbox series

Use `std::forward()` to forward the received arguments to enable the
potential use of move constructors instead of copy constructors.

Commit 0eacde623bb0 ("libcamera: object: Avoid argument copies in invokeMethod()")
added the forwarding references to `invokeMethod()` but it did not add the
appropriate `std::forward()` calls, so copying could still took place
even if not necessary.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 include/libcamera/base/object.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Hi Barnabás,

Thank you for the patch.

On Fri, Mar 28, 2025 at 10:52:23AM +0100, Barnabás Pőcze wrote:
> Use `std::forward()` to forward the received arguments to enable the
> potential use of move constructors instead of copy constructors.
> 
> Commit 0eacde623bb0 ("libcamera: object: Avoid argument copies in invokeMethod()")
> added the forwarding references to `invokeMethod()` but it did not add the
> appropriate `std::forward()` calls, so copying could still took place
> even if not necessary.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

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

> ---
>  include/libcamera/base/object.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/base/object.h b/include/libcamera/base/object.h
> index 6cb935a04..a24f84ff9 100644
> --- a/include/libcamera/base/object.h
> +++ b/include/libcamera/base/object.h
> @@ -9,6 +9,7 @@
>  
>  #include <list>
>  #include <memory>
> +#include <utility>
>  #include <vector>
>  
>  #include <libcamera/base/bound_method.h>
> @@ -39,7 +40,7 @@ public:
>  	{
>  		T *obj = static_cast<T *>(this);
>  		auto *method = new BoundMethodMember<T, R, FuncArgs...>(obj, this, func, type);
> -		return method->activate(args..., true);
> +		return method->activate(std::forward<Args>(args)..., true);
>  	}
>  
>  	Thread *thread() const { return thread_; }

Quoting Barnabás Pőcze (2025-03-28 09:52:23)
> Use `std::forward()` to forward the received arguments to enable the
> potential use of move constructors instead of copy constructors.
> 
> Commit 0eacde623bb0 ("libcamera: object: Avoid argument copies in invokeMethod()")
> added the forwarding references to `invokeMethod()` but it did not add the
> appropriate `std::forward()` calls, so copying could still took place

/took/take/


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

> even if not necessary.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  include/libcamera/base/object.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/base/object.h b/include/libcamera/base/object.h
> index 6cb935a04..a24f84ff9 100644
> --- a/include/libcamera/base/object.h
> +++ b/include/libcamera/base/object.h
> @@ -9,6 +9,7 @@
>  
>  #include <list>
>  #include <memory>
> +#include <utility>
>  #include <vector>
>  
>  #include <libcamera/base/bound_method.h>
> @@ -39,7 +40,7 @@ public:
>         {
>                 T *obj = static_cast<T *>(this);
>                 auto *method = new BoundMethodMember<T, R, FuncArgs...>(obj, this, func, type);
> -               return method->activate(args..., true);
> +               return method->activate(std::forward<Args>(args)..., true);
>         }
>  
>         Thread *thread() const { return thread_; }
> -- 
> 2.49.0
>