[libcamera-devel,1/3] libcamera: pipeline: raspberrypi: Don't leak RPiCameraData::sensor_

10656 diff mbox series

The CameraSensor instance stored in RPiCameraData::sensor_ is allocated
dynamically and never deleted. Fix the memory leak by storing it in a
std::unique_ptr<>.

Fixes: 740fd1b62f67 ("libcamera: pipeline: Raspberry Pi pipeline handler")
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Hi Laurent,

Thank you for your patch.


On Sat, 12 Dec 2020, 6:51 pm Laurent Pinchart, <
laurent.pinchart@ideasonboard.com> wrote:

> The CameraSensor instance stored in RPiCameraData::sensor_ is allocated
> dynamically and never deleted. Fix the memory leak by storing it in a
> std::unique_ptr<>.
>
> Fixes: 740fd1b62f67 ("libcamera: pipeline: Raspberry Pi pipeline handler")
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>

Reviewed-by: Naushir Patuck <naush@raspberrypi.com>


---
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 439c21ce4566..7a5f5881b9b3 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -7,6 +7,7 @@
>  #include <algorithm>
>  #include <assert.h>
>  #include <fcntl.h>
> +#include <memory>
>  #include <mutex>
>  #include <queue>
>  #include <sys/mman.h>
> @@ -134,7 +135,7 @@ class RPiCameraData : public CameraData
>  {
>  public:
>         RPiCameraData(PipelineHandler *pipe)
> -               : CameraData(pipe), sensor_(nullptr),
> state_(State::Stopped),
> +               : CameraData(pipe), state_(State::Stopped),
>                   supportsFlips_(false), flipsAlterBayerOrder_(false),
>                   updateScalerCrop_(true), dropFrameCount_(0),
> ispOutputCount_(0)
>         {
> @@ -158,7 +159,7 @@ public:
>         void handleState();
>         void applyScalerCrop(const ControlList &controls);
>
> -       CameraSensor *sensor_;
> +       std::unique_ptr<CameraSensor> sensor_;
>         /* Array of Unicam and ISP device streams and associated
> buffers/streams. */
>         RPi::Device<Unicam, 2> unicam_;
>         RPi::Device<Isp, 4> isp_;
> @@ -948,7 +949,7 @@ bool PipelineHandlerRPi::match(DeviceEnumerator
> *enumerator)
>         /* Identify the sensor. */
>         for (MediaEntity *entity : unicam_->entities()) {
>                 if (entity->function() == MEDIA_ENT_F_CAM_SENSOR) {
> -                       data->sensor_ = new CameraSensor(entity);
> +                       data->sensor_ =
> std::make_unique<CameraSensor>(entity);
>                         break;
>                 }
>         }
> --
> Regards,
>
> Laurent Pinchart
>
>

Hi Laurent,

On Sat, Dec 12, 2020 at 08:51:14PM +0200, Laurent Pinchart wrote:
> The CameraSensor instance stored in RPiCameraData::sensor_ is allocated
> dynamically and never deleted. Fix the memory leak by storing it in a
> std::unique_ptr<>.
> 
> Fixes: 740fd1b62f67 ("libcamera: pipeline: Raspberry Pi pipeline handler")
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 439c21ce4566..7a5f5881b9b3 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -7,6 +7,7 @@
>  #include <algorithm>
>  #include <assert.h>
>  #include <fcntl.h>
> +#include <memory>
>  #include <mutex>
>  #include <queue>
>  #include <sys/mman.h>
> @@ -134,7 +135,7 @@ class RPiCameraData : public CameraData
>  {
>  public:
>  	RPiCameraData(PipelineHandler *pipe)
> -		: CameraData(pipe), sensor_(nullptr), state_(State::Stopped),
> +		: CameraData(pipe), state_(State::Stopped),
>  		  supportsFlips_(false), flipsAlterBayerOrder_(false),
>  		  updateScalerCrop_(true), dropFrameCount_(0), ispOutputCount_(0)
>  	{
> @@ -158,7 +159,7 @@ public:
>  	void handleState();
>  	void applyScalerCrop(const ControlList &controls);
>  
> -	CameraSensor *sensor_;
> +	std::unique_ptr<CameraSensor> sensor_;
>  	/* Array of Unicam and ISP device streams and associated buffers/streams. */
>  	RPi::Device<Unicam, 2> unicam_;
>  	RPi::Device<Isp, 4> isp_;
> @@ -948,7 +949,7 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
>  	/* Identify the sensor. */
>  	for (MediaEntity *entity : unicam_->entities()) {
>  		if (entity->function() == MEDIA_ENT_F_CAM_SENSOR) {
> -			data->sensor_ = new CameraSensor(entity);
> +			data->sensor_ = std::make_unique<CameraSensor>(entity);
>  			break;
>  		}
>  	}
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel