[libcamera-devel] [PATCH v2 8/8] libcamera: camera_manager: Turn enumerator into a unique_ptr<>

[Niklas Söderlund]

Hi Laurent,

Thanks for your work.

On 2019-01-15 17:18:49 +0200, Laurent Pinchart wrote:
> Convey the fact that the CameraManager class owns the DeviceEnumerator
> instance it creates by using std::unique_ptr<> to store the pointer.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

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

> ---
>  include/libcamera/camera_manager.h        | 3 ++-
>  src/libcamera/camera_manager.cpp          | 8 ++------
>  src/libcamera/device_enumerator.cpp       | 9 ++++-----
>  src/libcamera/include/device_enumerator.h | 3 ++-
>  4 files changed, 10 insertions(+), 13 deletions(-)
> 
> diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h
> index 15e7c162032a..6cfcba3c3933 100644
> --- a/include/libcamera/camera_manager.h
> +++ b/include/libcamera/camera_manager.h
> @@ -7,6 +7,7 @@
>  #ifndef __LIBCAMERA_CAMERA_MANAGER_H__
>  #define __LIBCAMERA_CAMERA_MANAGER_H__
>  
> +#include <memory>
>  #include <string>
>  #include <vector>
>  
> @@ -37,7 +38,7 @@ private:
>  	void operator=(const CameraManager &) = delete;
>  	~CameraManager();
>  
> -	DeviceEnumerator *enumerator_;
> +	std::unique_ptr<DeviceEnumerator> enumerator_;
>  	std::vector<PipelineHandler *> pipes_;
>  
>  	EventDispatcher *dispatcher_;
> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> index a17bf3d13a04..ae5542bc332d 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -73,7 +73,6 @@ CameraManager::~CameraManager()
>   */
>  int CameraManager::start()
>  {
> -
>  	if (enumerator_)
>  		return -ENODEV;
>  
> @@ -95,7 +94,7 @@ int CameraManager::start()
>  		 */
>  		while (1) {
>  			PipelineHandler *pipe = factory->create();
> -			if (!pipe->match(enumerator_)) {
> +			if (!pipe->match(enumerator_.get())) {
>  				delete pipe;
>  				break;
>  			}
> @@ -130,10 +129,7 @@ void CameraManager::stop()
>  
>  	pipes_.clear();
>  
> -	if (enumerator_)
> -		delete enumerator_;
> -
> -	enumerator_ = nullptr;
> +	enumerator_.reset(nullptr);
>  }
>  
>  /**
> diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp
> index 18d7e86843e8..55c510e3b79a 100644
> --- a/src/libcamera/device_enumerator.cpp
> +++ b/src/libcamera/device_enumerator.cpp
> @@ -14,6 +14,7 @@
>  #include "device_enumerator.h"
>  #include "log.h"
>  #include "media_device.h"
> +#include "utils.h"
>  
>  /**
>   * \file device_enumerator.h
> @@ -128,20 +129,18 @@ bool DeviceMatch::match(const MediaDevice *device) const
>   * \return A pointer to the newly created device enumerator on success, or
>   * nullptr if an error occurs
>   */
> -DeviceEnumerator *DeviceEnumerator::create()
> +std::unique_ptr<DeviceEnumerator> DeviceEnumerator::create()
>  {
> -	DeviceEnumerator *enumerator;
> +	std::unique_ptr<DeviceEnumerator> enumerator;
>  
>  	/**
>  	 * \todo Add compile time checks to only try udev enumerator if libudev
>  	 * is available.
>  	 */
> -	enumerator = new DeviceEnumeratorUdev();
> +	enumerator = utils::make_unique<DeviceEnumeratorUdev>();
>  	if (!enumerator->init())
>  		return enumerator;
>  
> -	delete enumerator;
> -
>  	/*
>  	 * Either udev is not available or udev initialization failed. Fall back
>  	 * on the sysfs enumerator.
> diff --git a/src/libcamera/include/device_enumerator.h b/src/libcamera/include/device_enumerator.h
> index b68c815827dd..c5541c5f8d12 100644
> --- a/src/libcamera/include/device_enumerator.h
> +++ b/src/libcamera/include/device_enumerator.h
> @@ -8,6 +8,7 @@
>  #define __LIBCAMERA_DEVICE_ENUMERATOR_H__
>  
>  #include <map>
> +#include <memory>
>  #include <string>
>  #include <vector>
>  
> @@ -34,7 +35,7 @@ private:
>  class DeviceEnumerator
>  {
>  public:
> -	static DeviceEnumerator *create();
> +	static std::unique_ptr<DeviceEnumerator> create();
>  
>  	virtual ~DeviceEnumerator();
>  
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund