| Message ID | 20190115151849.1547-9-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
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@ideasonboard.com> Reviewed-by: Niklas Söderlund <niklas.soderlund@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@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
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();
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@ideasonboard.com> --- 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(-)