[libcamera-devel,v2,3/8] android: CameraHalManager: Fix a function call of a moved Camera
diff mbox series

Message ID 20210324070757.3530377-4-hiroh@chromium.org
State Accepted
Headers show
Series
  • Improve pointer types in android HAL adaptation layer
Related show

Commit Message

Hirokazu Honda March 24, 2021, 7:07 a.m. UTC 
libcamera::Camera::id() is called after std::move() in
cameraAdded(). This fixes the issue by not executing std::move().

Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
---
 src/android/camera_hal_manager.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--
2.31.0.291.g576ba9dcdaf-goog

Comments

Laurent Pinchart March 24, 2021, 9:01 p.m. UTC | #1 
Hi Hiro,

Thank you for the patch.

On Wed, Mar 24, 2021 at 04:07:52PM +0900, Hirokazu Honda wrote:
> libcamera::Camera::id() is called after std::move() in
> cameraAdded(). This fixes the issue by not executing std::move().
> 
> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>

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

> ---
>  src/android/camera_hal_manager.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
> index b501f8db..f70cd8a8 100644
> --- a/src/android/camera_hal_manager.cpp
> +++ b/src/android/camera_hal_manager.cpp
> @@ -115,7 +115,7 @@ void CameraHalManager::cameraAdded(std::shared_ptr<Camera> cam)
>  	}
> 
>  	/* Create a CameraDevice instance to wrap the libcamera Camera. */
> -	std::unique_ptr<CameraDevice> camera = CameraDevice::create(id, std::move(cam));
> +	std::unique_ptr<CameraDevice> camera = CameraDevice::create(id, cam);
>  	int ret = camera->initialize();
>  	if (ret) {
>  		LOG(HAL, Error) << "Failed to initialize camera: " << cam->id();
Jacopo Mondi March 25, 2021, 8:37 a.m. UTC | #2 
Hi Hiro,

On Wed, Mar 24, 2021 at 04:07:52PM +0900, Hirokazu Honda wrote:
> libcamera::Camera::id() is called after std::move() in
> cameraAdded(). This fixes the issue by not executing std::move().
>
> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> ---
>  src/android/camera_hal_manager.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
> index b501f8db..f70cd8a8 100644
> --- a/src/android/camera_hal_manager.cpp
> +++ b/src/android/camera_hal_manager.cpp
> @@ -115,7 +115,7 @@ void CameraHalManager::cameraAdded(std::shared_ptr<Camera> cam)
>  	}
>
>  	/* Create a CameraDevice instance to wrap the libcamera Camera. */
> -	std::unique_ptr<CameraDevice> camera = CameraDevice::create(id, std::move(cam));
> +	std::unique_ptr<CameraDevice> camera = CameraDevice::create(id, cam);

Furthermore, the shared pointer reference counter will be anyway
decreased when 'cam' goes out of scope.. So no need for move in any
way

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

Thanks
  j
>  	int ret = camera->initialize();
>  	if (ret) {
>  		LOG(HAL, Error) << "Failed to initialize camera: " << cam->id();
> --
> 2.31.0.291.g576ba9dcdaf-goog
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch
diff mbox series 

diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
index b501f8db..f70cd8a8 100644
--- a/src/android/camera_hal_manager.cpp
+++ b/src/android/camera_hal_manager.cpp
@@ -115,7 +115,7 @@  void CameraHalManager::cameraAdded(std::shared_ptr<Camera> cam)
 	}

 	/* Create a CameraDevice instance to wrap the libcamera Camera. */
-	std::unique_ptr<CameraDevice> camera = CameraDevice::create(id, std::move(cam));
+	std::unique_ptr<CameraDevice> camera = CameraDevice::create(id, cam);
 	int ret = camera->initialize();
 	if (ret) {
 		LOG(HAL, Error) << "Failed to initialize camera: " << cam->id();