[libcamera-devel] [PATCH 1/2] libcamera: v4l2_subdevice: Return a unique pointer from fromEntityName()
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Dec 8 02:49:50 CET 2020
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 at ideasonboard.com>
---
include/libcamera/internal/v4l2_subdevice.h | 5 +++--
src/libcamera/pipeline/ipu3/imgu.cpp | 2 +-
src/libcamera/pipeline/rkisp1/rkisp1.cpp | 6 ++----
src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 3 +--
src/libcamera/pipeline/rkisp1/rkisp1_path.h | 3 ++-
src/libcamera/v4l2_subdevice.cpp | 10 ++++------
6 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h
index 02ee3e914a8b..eb25fa2fd01b 100644
--- a/include/libcamera/internal/v4l2_subdevice.h
+++ b/include/libcamera/internal/v4l2_subdevice.h
@@ -7,6 +7,7 @@
#ifndef __LIBCAMERA_INTERNAL_V4L2_SUBDEVICE_H__
#define __LIBCAMERA_INTERNAL_V4L2_SUBDEVICE_H__
+#include <memory>
#include <string>
#include <vector>
@@ -60,8 +61,8 @@ public:
int setFormat(unsigned int pad, V4L2SubdeviceFormat *format,
Whence whence = ActiveFormat);
- static V4L2Subdevice *fromEntityName(const MediaDevice *media,
- const std::string &entity);
+ static std::unique_ptr<V4L2Subdevice>
+ fromEntityName(const MediaDevice *media, const std::string &entity);
protected:
std::string logPrefix() const override;
diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp
index a4d74a62f69a..bfe9624c7797 100644
--- a/src/libcamera/pipeline/ipu3/imgu.cpp
+++ b/src/libcamera/pipeline/ipu3/imgu.cpp
@@ -343,7 +343,7 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index)
* by the match() function: no need to check for newly created
* video devices and subdevice validity here.
*/
- imgu_.reset(V4L2Subdevice::fromEntityName(media, name_));
+ imgu_ = V4L2Subdevice::fromEntityName(media, name_);
ret = imgu_->open();
if (ret)
return ret;
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 6e74a49abfda..bcfe6c0514ab 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -217,7 +217,7 @@ private:
int freeBuffers(Camera *camera);
MediaDevice *media_;
- V4L2Subdevice *isp_;
+ std::unique_ptr<V4L2Subdevice> isp_;
V4L2VideoDevice *param_;
V4L2VideoDevice *stat_;
@@ -599,8 +599,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
}
PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)
- : PipelineHandler(manager), isp_(nullptr), param_(nullptr),
- stat_(nullptr)
+ : PipelineHandler(manager), param_(nullptr), stat_(nullptr)
{
}
@@ -608,7 +607,6 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1()
{
delete param_;
delete stat_;
- delete isp_;
}
/* -----------------------------------------------------------------------------
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
index 3f77b1c12f9d..e05d9dd657cd 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
@@ -24,14 +24,13 @@ RkISP1Path::RkISP1Path(const char *name, const Span<const PixelFormat> &formats,
const Size &minResolution, const Size &maxResolution)
: name_(name), running_(false), formats_(formats),
minResolution_(minResolution), maxResolution_(maxResolution),
- resizer_(nullptr), video_(nullptr), link_(nullptr)
+ video_(nullptr), link_(nullptr)
{
}
RkISP1Path::~RkISP1Path()
{
delete video_;
- delete resizer_;
}
bool RkISP1Path::init(MediaDevice *media)
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
index 8f443e5179f6..f06ac5a731cc 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h
+++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
@@ -7,6 +7,7 @@
#ifndef __LIBCAMERA_PIPELINE_RKISP1_PATH_H__
#define __LIBCAMERA_PIPELINE_RKISP1_PATH_H__
+#include <memory>
#include <vector>
#include <libcamera/camera.h>
@@ -65,7 +66,7 @@ private:
const Size minResolution_;
const Size maxResolution_;
- V4L2Subdevice *resizer_;
+ std::unique_ptr<V4L2Subdevice> resizer_;
V4L2VideoDevice *video_;
MediaLink *link_;
};
diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
index 85d00c246f5e..721ff5a92a2b 100644
--- a/src/libcamera/v4l2_subdevice.cpp
+++ b/src/libcamera/v4l2_subdevice.cpp
@@ -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] 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
*/
-V4L2Subdevice *V4L2Subdevice::fromEntityName(const MediaDevice *media,
- const std::string &entity)
+std::unique_ptr<V4L2Subdevice>
+V4L2Subdevice::fromEntityName(const MediaDevice *media,
+ const std::string &entity)
{
MediaEntity *mediaEntity = media->getEntityByName(entity);
if (!mediaEntity)
return nullptr;
- return new V4L2Subdevice(mediaEntity);
+ return std::make_unique<V4L2Subdevice>(mediaEntity);
}
std::string V4L2Subdevice::logPrefix() const
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list