[PATCH v1 2/2] gstreamer: allocator: gst_libcamera_allocator_new(): Fix memory leak

[Laurent Pinchart]

Hi Barnabás,

Thank you for the patch.

On Sun, Dec 15, 2024 at 06:18:44PM +0000, Barnabás Pőcze wrote:
> If `FrameBufferAllocator::allocate()` causes the construction to be
> aborted, the allocated `GstLibcameraAllocator` will not be
> deallocated properly. Use `g_autoptr()` to address this.
> 
> `g_steal_pointer()` cannot be used because that evaluates to
> an expression of type `void *`, so one would need an extra static_cast.
> Instead, use `std::exchange()`.
> 
> Signed-off-by: Barnabás Pőcze <pobrn at protonmail.com>
> ---
>  src/gstreamer/gstlibcameraallocator.cpp | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/src/gstreamer/gstlibcameraallocator.cpp b/src/gstreamer/gstlibcameraallocator.cpp
> index b0c84893a..341bd31b3 100644
> --- a/src/gstreamer/gstlibcameraallocator.cpp
> +++ b/src/gstreamer/gstlibcameraallocator.cpp
> @@ -199,15 +199,13 @@ GstLibcameraAllocator *
>  gst_libcamera_allocator_new(std::shared_ptr<Camera> camera,
>  			    CameraConfiguration *config_)
>  {
> -	auto *self = GST_LIBCAMERA_ALLOCATOR(g_object_new(GST_TYPE_LIBCAMERA_ALLOCATOR,
> -							  nullptr));
> +	g_autoptr(GstLibcameraAllocator) self = GST_LIBCAMERA_ALLOCATOR(g_object_new(GST_TYPE_LIBCAMERA_ALLOCATOR,
> +										     nullptr));
>  	gint ret;
>  
>  	self->cm_ptr = new std::shared_ptr<CameraManager>(gst_libcamera_get_camera_manager(ret));
> -	if (ret) {
> -		g_object_unref(self);
> +	if (ret)
>  		return nullptr;
> -	}
>  
>  	self->fb_allocator = new FrameBufferAllocator(camera);
>  	for (StreamConfiguration &streamCfg : *config_) {
> @@ -228,7 +226,7 @@ gst_libcamera_allocator_new(std::shared_ptr<Camera> camera,
>  		g_hash_table_insert(self->pools, stream, pool);
>  	}
>  
> -	return self;
> +	return std::exchange(self, nullptr);

#include <utility>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

>  }
>  
>  bool

-- 
Regards,

Laurent Pinchart