[v2,5/7] gstreamer: Fix leak of GstQuery and GstBufferPool in error path
diff mbox series

Message ID 20250604130741.9228-6-laurent.pinchart@ideasonboard.com
State New
Headers show
Series
  • gstreamer: Miscellaneous cleanups + two fixes
Related show

Commit Message

Laurent Pinchart June 4, 2025, 1:07 p.m. UTC
The gst_libcamera_create_video_pool() function leaks a GstQuery instance
and a GstBufferPool instance in an error path. Fix the leaks with
g_autoptr().

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/gstreamer/gstlibcamerasrc.cpp | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

Barnabás Pőcze June 4, 2025, 1:36 p.m. UTC | #1
Hi


2025. 06. 04. 15:07 keltezéssel, Laurent Pinchart írta:
> The gst_libcamera_create_video_pool() function leaks a GstQuery instance
> and a GstBufferPool instance in an error path. Fix the leaks with
> g_autoptr().
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>   src/gstreamer/gstlibcamerasrc.cpp | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index 9c0ee491ab63..0be64dd836ca 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -541,9 +541,9 @@ static std::tuple<GstBufferPool *, int>
>   gst_libcamera_create_video_pool(GstLibcameraSrc *self, GstPad *srcpad,
>   				GstCaps *caps, const GstVideoInfo *info)
>   {
> -	GstQuery *query = NULL;
> +	g_autoptr(GstQuery) query = NULL;
> +	g_autoptr(GstBufferPool) pool = NULL;
>   	const gboolean need_pool = true;
> -	GstBufferPool *pool = NULL;
>   
>   	/*
>   	 * Get the peer allocation hints to check if it supports the meta API.
> @@ -587,8 +587,7 @@ gst_libcamera_create_video_pool(GstLibcameraSrc *self, GstPad *srcpad,
>   		return { NULL, -EINVAL };
>   	}
>   
> -	gst_query_unref(query);
> -	return { pool, 0 };
> +	return { reinterpret_cast<GstBufferPool *>(g_steal_pointer(&pool)), 0 };

`std::exchange(pool, nullptr)` ? I used that in https://patchwork.libcamera.org/patch/22455/
because `g_steal_pointer()` only preserves the type since glib 2.68.


Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>


Regards,
Barnabás Pőcze


>   }
>   
>   /* Must be called with stream_lock held. */
Laurent Pinchart June 4, 2025, 1:41 p.m. UTC | #2
On Wed, Jun 04, 2025 at 03:36:34PM +0200, Barnabás Pőcze wrote:
> 2025. 06. 04. 15:07 keltezéssel, Laurent Pinchart írta:
> > The gst_libcamera_create_video_pool() function leaks a GstQuery instance
> > and a GstBufferPool instance in an error path. Fix the leaks with
> > g_autoptr().
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >   src/gstreamer/gstlibcamerasrc.cpp | 7 +++----
> >   1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> > index 9c0ee491ab63..0be64dd836ca 100644
> > --- a/src/gstreamer/gstlibcamerasrc.cpp
> > +++ b/src/gstreamer/gstlibcamerasrc.cpp
> > @@ -541,9 +541,9 @@ static std::tuple<GstBufferPool *, int>
> >   gst_libcamera_create_video_pool(GstLibcameraSrc *self, GstPad *srcpad,
> >   				GstCaps *caps, const GstVideoInfo *info)
> >   {
> > -	GstQuery *query = NULL;
> > +	g_autoptr(GstQuery) query = NULL;
> > +	g_autoptr(GstBufferPool) pool = NULL;
> >   	const gboolean need_pool = true;
> > -	GstBufferPool *pool = NULL;
> >   
> >   	/*
> >   	 * Get the peer allocation hints to check if it supports the meta API.
> > @@ -587,8 +587,7 @@ gst_libcamera_create_video_pool(GstLibcameraSrc *self, GstPad *srcpad,
> >   		return { NULL, -EINVAL };
> >   	}
> >   
> > -	gst_query_unref(query);
> > -	return { pool, 0 };
> > +	return { reinterpret_cast<GstBufferPool *>(g_steal_pointer(&pool)), 0 };
> 
> `std::exchange(pool, nullptr)` ? I used that in https://patchwork.libcamera.org/patch/22455/
> because `g_steal_pointer()` only preserves the type since glib 2.68.

Good idea. I'll use that.

> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> 
> >   }
> >   
> >   /* Must be called with stream_lock held. */

Patch
diff mbox series

diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
index 9c0ee491ab63..0be64dd836ca 100644
--- a/src/gstreamer/gstlibcamerasrc.cpp
+++ b/src/gstreamer/gstlibcamerasrc.cpp
@@ -541,9 +541,9 @@  static std::tuple<GstBufferPool *, int>
 gst_libcamera_create_video_pool(GstLibcameraSrc *self, GstPad *srcpad,
 				GstCaps *caps, const GstVideoInfo *info)
 {
-	GstQuery *query = NULL;
+	g_autoptr(GstQuery) query = NULL;
+	g_autoptr(GstBufferPool) pool = NULL;
 	const gboolean need_pool = true;
-	GstBufferPool *pool = NULL;
 
 	/*
 	 * Get the peer allocation hints to check if it supports the meta API.
@@ -587,8 +587,7 @@  gst_libcamera_create_video_pool(GstLibcameraSrc *self, GstPad *srcpad,
 		return { NULL, -EINVAL };
 	}
 
-	gst_query_unref(query);
-	return { pool, 0 };
+	return { reinterpret_cast<GstBufferPool *>(g_steal_pointer(&pool)), 0 };
 }
 
 /* Must be called with stream_lock held. */