libcamera: v4l2_subdevice: Return a unique pointer from fromEntityName()

The fromEntityName() function returns a pointer to a newly allocated
V4L2Subdevice instance, which must be deleted by the caller. This opens
the door to memory leaks. Return a unique pointer instead, which conveys
the API semantics better than a sentence in the documentation.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
This commit is contained in:
Laurent Pinchart 2020-12-08 03:40:25 +02:00
parent 2795f333fc
commit ff2ee0174c
6 changed files with 13 additions and 16 deletions

View file

@ -7,6 +7,7 @@
#ifndef __LIBCAMERA_INTERNAL_V4L2_SUBDEVICE_H__ #ifndef __LIBCAMERA_INTERNAL_V4L2_SUBDEVICE_H__
#define __LIBCAMERA_INTERNAL_V4L2_SUBDEVICE_H__ #define __LIBCAMERA_INTERNAL_V4L2_SUBDEVICE_H__
#include <memory>
#include <string> #include <string>
#include <vector> #include <vector>
@ -60,8 +61,8 @@ public:
int setFormat(unsigned int pad, V4L2SubdeviceFormat *format, int setFormat(unsigned int pad, V4L2SubdeviceFormat *format,
Whence whence = ActiveFormat); Whence whence = ActiveFormat);
static V4L2Subdevice *fromEntityName(const MediaDevice *media, static std::unique_ptr<V4L2Subdevice>
const std::string &entity); fromEntityName(const MediaDevice *media, const std::string &entity);
protected: protected:
std::string logPrefix() const override; std::string logPrefix() const override;

View file

@ -343,7 +343,7 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index)
* by the match() function: no need to check for newly created * by the match() function: no need to check for newly created
* video devices and subdevice validity here. * video devices and subdevice validity here.
*/ */
imgu_.reset(V4L2Subdevice::fromEntityName(media, name_)); imgu_ = V4L2Subdevice::fromEntityName(media, name_);
ret = imgu_->open(); ret = imgu_->open();
if (ret) if (ret)
return ret; return ret;

View file

@ -217,7 +217,7 @@ private:
int freeBuffers(Camera *camera); int freeBuffers(Camera *camera);
MediaDevice *media_; MediaDevice *media_;
V4L2Subdevice *isp_; std::unique_ptr<V4L2Subdevice> isp_;
V4L2VideoDevice *param_; V4L2VideoDevice *param_;
V4L2VideoDevice *stat_; V4L2VideoDevice *stat_;
@ -599,8 +599,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
} }
PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager) PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)
: PipelineHandler(manager), isp_(nullptr), param_(nullptr), : PipelineHandler(manager), param_(nullptr), stat_(nullptr)
stat_(nullptr)
{ {
} }
@ -608,7 +607,6 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1()
{ {
delete param_; delete param_;
delete stat_; delete stat_;
delete isp_;
} }
/* ----------------------------------------------------------------------------- /* -----------------------------------------------------------------------------

View file

@ -24,14 +24,13 @@ RkISP1Path::RkISP1Path(const char *name, const Span<const PixelFormat> &formats,
const Size &minResolution, const Size &maxResolution) const Size &minResolution, const Size &maxResolution)
: name_(name), running_(false), formats_(formats), : name_(name), running_(false), formats_(formats),
minResolution_(minResolution), maxResolution_(maxResolution), minResolution_(minResolution), maxResolution_(maxResolution),
resizer_(nullptr), video_(nullptr), link_(nullptr) video_(nullptr), link_(nullptr)
{ {
} }
RkISP1Path::~RkISP1Path() RkISP1Path::~RkISP1Path()
{ {
delete video_; delete video_;
delete resizer_;
} }
bool RkISP1Path::init(MediaDevice *media) bool RkISP1Path::init(MediaDevice *media)

View file

@ -7,6 +7,7 @@
#ifndef __LIBCAMERA_PIPELINE_RKISP1_PATH_H__ #ifndef __LIBCAMERA_PIPELINE_RKISP1_PATH_H__
#define __LIBCAMERA_PIPELINE_RKISP1_PATH_H__ #define __LIBCAMERA_PIPELINE_RKISP1_PATH_H__
#include <memory>
#include <vector> #include <vector>
#include <libcamera/camera.h> #include <libcamera/camera.h>
@ -65,7 +66,7 @@ private:
const Size minResolution_; const Size minResolution_;
const Size maxResolution_; const Size maxResolution_;
V4L2Subdevice *resizer_; std::unique_ptr<V4L2Subdevice> resizer_;
V4L2VideoDevice *video_; V4L2VideoDevice *video_;
MediaLink *link_; MediaLink *link_;
}; };

View file

@ -446,19 +446,17 @@ int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,
* \param[in] media The media device where the entity is registered * \param[in] media The media device where the entity is registered
* \param[in] entity The media entity name * \param[in] entity The media entity name
* *
* Releasing memory of the newly created instance is responsibility of the
* caller of this function.
*
* \return A newly created V4L2Subdevice on success, nullptr otherwise * \return A newly created V4L2Subdevice on success, nullptr otherwise
*/ */
V4L2Subdevice *V4L2Subdevice::fromEntityName(const MediaDevice *media, std::unique_ptr<V4L2Subdevice>
V4L2Subdevice::fromEntityName(const MediaDevice *media,
const std::string &entity) const std::string &entity)
{ {
MediaEntity *mediaEntity = media->getEntityByName(entity); MediaEntity *mediaEntity = media->getEntityByName(entity);
if (!mediaEntity) if (!mediaEntity)
return nullptr; return nullptr;
return new V4L2Subdevice(mediaEntity); return std::make_unique<V4L2Subdevice>(mediaEntity);
} }
std::string V4L2Subdevice::logPrefix() const std::string V4L2Subdevice::logPrefix() const