[libcamera-devel] [PATCH v1.1 2/4] libcamera: camera_manager: Use std::unique_ptr to store event dispatcher

[Niklas Söderlund]

Hi Laurent,

Thanks for your patch.

On 2019-01-18 02:32:01 +0200, Laurent Pinchart wrote:
> The CameraManager takes ownership of the dispatcher passed to the
> setEventDispatcher() function. Enforces this by using std::unique_ptr<>.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

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

> ---
> Changes since v1:
> 
> - Use make_unique<> in CameraManager::setEventDispatcher()
> ---
>  include/libcamera/camera_manager.h |  4 ++--
>  src/libcamera/camera_manager.cpp   | 12 +++++++-----
>  2 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h
> index 6cfcba3c3933..b82a8ce95b9f 100644
> --- a/include/libcamera/camera_manager.h
> +++ b/include/libcamera/camera_manager.h
> @@ -29,7 +29,7 @@ public:
>  
>  	static CameraManager *instance();
>  
> -	void setEventDispatcher(EventDispatcher *dispatcher);
> +	void setEventDispatcher(std::unique_ptr<EventDispatcher> dispatcher);
>  	EventDispatcher *eventDispatcher();
>  
>  private:
> @@ -41,7 +41,7 @@ private:
>  	std::unique_ptr<DeviceEnumerator> enumerator_;
>  	std::vector<PipelineHandler *> pipes_;
>  
> -	EventDispatcher *dispatcher_;
> +	std::unique_ptr<EventDispatcher> dispatcher_;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> index 9f554be57e33..1430bb0d75a5 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -12,6 +12,7 @@
>  #include "event_dispatcher_poll.h"
>  #include "log.h"
>  #include "pipeline_handler.h"
> +#include "utils.h"
>  
>  /**
>   * \file camera_manager.h
> @@ -58,7 +59,6 @@ CameraManager::CameraManager()
>  
>  CameraManager::~CameraManager()
>  {
> -	delete dispatcher_;
>  }
>  
>  /**
> @@ -209,14 +209,14 @@ CameraManager *CameraManager::instance()
>   * The CameraManager takes ownership of the event dispatcher and will delete it
>   * when the application terminates.
>   */
> -void CameraManager::setEventDispatcher(EventDispatcher *dispatcher)
> +void CameraManager::setEventDispatcher(std::unique_ptr<EventDispatcher> dispatcher)
>  {
>  	if (dispatcher_) {
>  		LOG(Warning) << "Event dispatcher is already set";
>  		return;
>  	}
>  
> -	dispatcher_ = dispatcher;
> +	dispatcher_ = std::move(dispatcher);
>  }
>  
>  /**
> @@ -226,14 +226,16 @@ void CameraManager::setEventDispatcher(EventDispatcher *dispatcher)
>   * If no dispatcher has been set, a default poll-based implementation is created
>   * and returned, and no custom event dispatcher may be installed anymore.
>   *
> + * The returned event dispatcher is valid until the camera manager is destroyed.
> + *
>   * \return Pointer to the event dispatcher
>   */
>  EventDispatcher *CameraManager::eventDispatcher()
>  {
>  	if (!dispatcher_)
> -		dispatcher_ = new EventDispatcherPoll();
> +		dispatcher_ = utils::make_unique<EventDispatcherPoll>();
>  
> -	return dispatcher_;
> +	return dispatcher_.get();
>  }
>  
>  } /* namespace libcamera */
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund