[libcamera-devel,1/2] gst: provider: Fix crash in finalize
diff mbox series

Message ID 20210311205255.741985-2-nicolas@ndufresne.ca
State Accepted
Commit 488bbe40a9183a6061e4e3fc975d8e289c6f75ac
Headers show
Series
  • GStreamer Fixes
Related show

Commit Message

Nicolas Dufresne March 11, 2021, 8:52 p.m. UTC 
From: Nicolas Dufresne <nicolas.dufresne@collabora.com>

Both the DeviceProvider and Device classes had the same mistake,
calling G_OBJECT_GET_CLASS() instead of G_OBJECT_CLASS() when
chaining their finalize call to their base class. This would
crash at destruction, which was causing gst-device-monitor-1.0
tool to crash and application using that API to crash too.

Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 src/gstreamer/gstlibcameraprovider.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Laurent Pinchart March 11, 2021, 10:39 p.m. UTC | #1 
Hi Nicolas,

Thank you for the patch.

On Thu, Mar 11, 2021 at 03:52:54PM -0500, Nicolas Dufresne wrote:
> From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> 
> Both the DeviceProvider and Device classes had the same mistake,
> calling G_OBJECT_GET_CLASS() instead of G_OBJECT_CLASS() when
> chaining their finalize call to their base class. This would
> crash at destruction, which was causing gst-device-monitor-1.0
> tool to crash and application using that API to crash too.
> 
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

This looks good to me,

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

A bit annoying that gobject can't catch this type of issue at compile
time :-S

> ---
>  src/gstreamer/gstlibcameraprovider.cpp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp
> index ee44dc73..29da6c32 100644
> --- a/src/gstreamer/gstlibcameraprovider.cpp
> +++ b/src/gstreamer/gstlibcameraprovider.cpp
> @@ -101,7 +101,7 @@ gst_libcamera_device_finalize(GObject *object)
>  
>  	g_free(self->name);
>  
> -	G_OBJECT_GET_CLASS(klass)->finalize(object);
> +	G_OBJECT_CLASS(klass)->finalize(object);
>  }
>  
>  static void
> @@ -218,7 +218,7 @@ gst_libcamera_provider_finalize(GObject *object)
>  
>  	delete self->cm;
>  
> -	return G_OBJECT_GET_CLASS(klass)->finalize(object);
> +	return G_OBJECT_CLASS(klass)->finalize(object);
>  }
>  
>  static void
Umang Jain March 12, 2021, 5:05 a.m. UTC | #2 
Hi Nicolas,

Thank you for the patch.

On 3/12/21 2:22 AM, Nicolas Dufresne wrote:
> From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
>
> Both the DeviceProvider and Device classes had the same mistake,
> calling G_OBJECT_GET_CLASS() instead of G_OBJECT_CLASS() when
> chaining their finalize call to their base class. This would
> crash at destruction, which was causing gst-device-monitor-1.0
> tool to crash and application using that API to crash too.
>
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Reviewed-by: Umang Jain <email@uajain.com>
> ---
>   src/gstreamer/gstlibcameraprovider.cpp | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp
> index ee44dc73..29da6c32 100644
> --- a/src/gstreamer/gstlibcameraprovider.cpp
> +++ b/src/gstreamer/gstlibcameraprovider.cpp
> @@ -101,7 +101,7 @@ gst_libcamera_device_finalize(GObject *object)
>   
>   	g_free(self->name);
>   
> -	G_OBJECT_GET_CLASS(klass)->finalize(object);
> +	G_OBJECT_CLASS(klass)->finalize(object);
>   }
>   
>   static void
> @@ -218,7 +218,7 @@ gst_libcamera_provider_finalize(GObject *object)
>   
>   	delete self->cm;
>   
> -	return G_OBJECT_GET_CLASS(klass)->finalize(object);
> +	return G_OBJECT_CLASS(klass)->finalize(object);
>   }
>   
>   static void
Nicolas Dufresne March 12, 2021, 4:44 p.m. UTC | #3 
Le vendredi 12 mars 2021 à 00:39 +0200, Laurent Pinchart a écrit :
> Hi Nicolas,
> 
> Thank you for the patch.
> 
> On Thu, Mar 11, 2021 at 03:52:54PM -0500, Nicolas Dufresne wrote:
> > From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > 
> > Both the DeviceProvider and Device classes had the same mistake,
> > calling G_OBJECT_GET_CLASS() instead of G_OBJECT_CLASS() when
> > chaining their finalize call to their base class. This would
> > crash at destruction, which was causing gst-device-monitor-1.0
> > tool to crash and application using that API to crash too.
> > 
> > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> 
> This looks good to me,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> A bit annoying that gobject can't catch this type of issue at compile
> time :-S

Indeed, and it didn't at runtime either, looks like a gap in the runtime checks.
Compile time of course would require compiler support.

> 
> > ---
> >  src/gstreamer/gstlibcameraprovider.cpp | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/gstreamer/gstlibcameraprovider.cpp
> > b/src/gstreamer/gstlibcameraprovider.cpp
> > index ee44dc73..29da6c32 100644
> > --- a/src/gstreamer/gstlibcameraprovider.cpp
> > +++ b/src/gstreamer/gstlibcameraprovider.cpp
> > @@ -101,7 +101,7 @@ gst_libcamera_device_finalize(GObject *object)
> >  
> >         g_free(self->name);
> >  
> > -       G_OBJECT_GET_CLASS(klass)->finalize(object);
> > +       G_OBJECT_CLASS(klass)->finalize(object);
> >  }
> >  
> >  static void
> > @@ -218,7 +218,7 @@ gst_libcamera_provider_finalize(GObject *object)
> >  
> >         delete self->cm;
> >  
> > -       return G_OBJECT_GET_CLASS(klass)->finalize(object);
> > +       return G_OBJECT_CLASS(klass)->finalize(object);
> >  }
> >  
> >  static void
>

Patch
diff mbox series 

diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp
index ee44dc73..29da6c32 100644
--- a/src/gstreamer/gstlibcameraprovider.cpp
+++ b/src/gstreamer/gstlibcameraprovider.cpp
@@ -101,7 +101,7 @@  gst_libcamera_device_finalize(GObject *object)
 
 	g_free(self->name);
 
-	G_OBJECT_GET_CLASS(klass)->finalize(object);
+	G_OBJECT_CLASS(klass)->finalize(object);
 }
 
 static void
@@ -218,7 +218,7 @@  gst_libcamera_provider_finalize(GObject *object)
 
 	delete self->cm;
 
-	return G_OBJECT_GET_CLASS(klass)->finalize(object);
+	return G_OBJECT_CLASS(klass)->finalize(object);
 }
 
 static void