[libcamera-devel] [PATCH 2/2] libcamera: v4l2_device: Return a unique pointer from fromEntityName()
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Dec 8 02:49:51 CET 2020
The fromEntityName() function returns a pointer to a newly allocated
V4L2Device 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_videodevice.h | 4 ++--
src/libcamera/pipeline/ipu3/cio2.cpp | 3 +--
src/libcamera/pipeline/ipu3/cio2.h | 2 +-
src/libcamera/pipeline/ipu3/imgu.cpp | 10 ++++------
src/libcamera/pipeline/rkisp1/rkisp1.cpp | 13 +++----------
src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 7 +------
src/libcamera/pipeline/rkisp1/rkisp1_path.h | 3 +--
src/libcamera/v4l2_videodevice.cpp | 10 ++++------
test/libtest/buffer_source.cpp | 2 +-
9 files changed, 18 insertions(+), 36 deletions(-)
diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
index 661503d1565a..529ca0e359d6 100644
--- a/include/libcamera/internal/v4l2_videodevice.h
+++ b/include/libcamera/internal/v4l2_videodevice.h
@@ -210,8 +210,8 @@ public:
int streamOn();
int streamOff();
- static V4L2VideoDevice *fromEntityName(const MediaDevice *media,
- const std::string &entity);
+ static std::unique_ptr<V4L2VideoDevice>
+ fromEntityName(const MediaDevice *media, const std::string &entity);
V4L2PixelFormat toV4L2PixelFormat(const PixelFormat &pixelFormat);
diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
index e43ec70fe3e4..821715e325b4 100644
--- a/src/libcamera/pipeline/ipu3/cio2.cpp
+++ b/src/libcamera/pipeline/ipu3/cio2.cpp
@@ -33,13 +33,12 @@ const std::map<uint32_t, PixelFormat> mbusCodesToPixelFormat = {
} /* namespace */
CIO2Device::CIO2Device()
- : sensor_(nullptr), csi2_(nullptr), output_(nullptr)
+ : sensor_(nullptr), csi2_(nullptr)
{
}
CIO2Device::~CIO2Device()
{
- delete output_;
delete csi2_;
delete sensor_;
}
diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h
index fa813a989fd2..0dca9673ca07 100644
--- a/src/libcamera/pipeline/ipu3/cio2.h
+++ b/src/libcamera/pipeline/ipu3/cio2.h
@@ -63,7 +63,7 @@ private:
CameraSensor *sensor_;
V4L2Subdevice *csi2_;
- V4L2VideoDevice *output_;
+ std::unique_ptr<V4L2VideoDevice> output_;
std::vector<std::unique_ptr<FrameBuffer>> buffers_;
std::queue<FrameBuffer *> availableBuffers_;
diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp
index bfe9624c7797..5b1c0318b426 100644
--- a/src/libcamera/pipeline/ipu3/imgu.cpp
+++ b/src/libcamera/pipeline/ipu3/imgu.cpp
@@ -348,24 +348,22 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index)
if (ret)
return ret;
- input_.reset(V4L2VideoDevice::fromEntityName(media, name_ + " input"));
+ input_ = V4L2VideoDevice::fromEntityName(media, name_ + " input");
ret = input_->open();
if (ret)
return ret;
- output_.reset(V4L2VideoDevice::fromEntityName(media,
- name_ + " output"));
+ output_ = V4L2VideoDevice::fromEntityName(media, name_ + " output");
ret = output_->open();
if (ret)
return ret;
- viewfinder_.reset(V4L2VideoDevice::fromEntityName(media,
- name_ + " viewfinder"));
+ viewfinder_ = V4L2VideoDevice::fromEntityName(media, name_ + " viewfinder");
ret = viewfinder_->open();
if (ret)
return ret;
- stat_.reset(V4L2VideoDevice::fromEntityName(media, name_ + " 3a stat"));
+ stat_ = V4L2VideoDevice::fromEntityName(media, name_ + " 3a stat");
ret = stat_->open();
if (ret)
return ret;
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index bcfe6c0514ab..064ab01057f4 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -178,7 +178,6 @@ class PipelineHandlerRkISP1 : public PipelineHandler
{
public:
PipelineHandlerRkISP1(CameraManager *manager);
- ~PipelineHandlerRkISP1();
CameraConfiguration *generateConfiguration(Camera *camera,
const StreamRoles &roles) override;
@@ -218,8 +217,8 @@ private:
MediaDevice *media_;
std::unique_ptr<V4L2Subdevice> isp_;
- V4L2VideoDevice *param_;
- V4L2VideoDevice *stat_;
+ std::unique_ptr<V4L2VideoDevice> param_;
+ std::unique_ptr<V4L2VideoDevice> stat_;
RkISP1MainPath mainPath_;
RkISP1SelfPath selfPath_;
@@ -599,16 +598,10 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
}
PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)
- : PipelineHandler(manager), param_(nullptr), stat_(nullptr)
+ : PipelineHandler(manager)
{
}
-PipelineHandlerRkISP1::~PipelineHandlerRkISP1()
-{
- delete param_;
- delete stat_;
-}
-
/* -----------------------------------------------------------------------------
* Pipeline Operations
*/
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
index e05d9dd657cd..25f482eb8d8e 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
@@ -24,15 +24,10 @@ 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),
- video_(nullptr), link_(nullptr)
+ link_(nullptr)
{
}
-RkISP1Path::~RkISP1Path()
-{
- delete video_;
-}
-
bool RkISP1Path::init(MediaDevice *media)
{
std::string resizer = std::string("rkisp1_resizer_") + name_ + "path";
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
index f06ac5a731cc..3b3e37d258d0 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h
+++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
@@ -31,7 +31,6 @@ class RkISP1Path
public:
RkISP1Path(const char *name, const Span<const PixelFormat> &formats,
const Size &minResolution, const Size &maxResolution);
- ~RkISP1Path();
bool init(MediaDevice *media);
@@ -67,7 +66,7 @@ private:
const Size maxResolution_;
std::unique_ptr<V4L2Subdevice> resizer_;
- V4L2VideoDevice *video_;
+ std::unique_ptr<V4L2VideoDevice> video_;
MediaLink *link_;
};
diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
index e76fe2dd1f3a..e2b582842a9b 100644
--- a/src/libcamera/v4l2_videodevice.cpp
+++ b/src/libcamera/v4l2_videodevice.cpp
@@ -1598,19 +1598,17 @@ int V4L2VideoDevice::streamOff()
* \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 V4L2VideoDevice on success, nullptr otherwise
*/
-V4L2VideoDevice *V4L2VideoDevice::fromEntityName(const MediaDevice *media,
- const std::string &entity)
+std::unique_ptr<V4L2VideoDevice>
+V4L2VideoDevice::fromEntityName(const MediaDevice *media,
+ const std::string &entity)
{
MediaEntity *mediaEntity = media->getEntityByName(entity);
if (!mediaEntity)
return nullptr;
- return new V4L2VideoDevice(mediaEntity);
+ return std::make_unique<V4L2VideoDevice>(mediaEntity);
}
/**
diff --git a/test/libtest/buffer_source.cpp b/test/libtest/buffer_source.cpp
index ee87c8cd821c..73563f2fc39d 100644
--- a/test/libtest/buffer_source.cpp
+++ b/test/libtest/buffer_source.cpp
@@ -50,7 +50,7 @@ int BufferSource::allocate(const StreamConfiguration &config)
return TestSkip;
}
- std::unique_ptr<V4L2VideoDevice> video{ V4L2VideoDevice::fromEntityName(media_.get(), videoDeviceName) };
+ std::unique_ptr<V4L2VideoDevice> video = V4L2VideoDevice::fromEntityName(media_.get(), videoDeviceName);
if (!video) {
std::cout << "Failed to get video device from entity "
<< videoDeviceName << std::endl;
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list