[libcamera-devel] tests: gstreamer: Fix compiler error with gcc 8.4.0
diff mbox series

Message ID 20230707090249.21635-1-laurent.pinchart@ideasonboard.com
State Accepted
Commit 03526e58d1fc58012374ad83b43c93a9d3ea289f
Headers show
Series
  • [libcamera-devel] tests: gstreamer: Fix compiler error with gcc 8.4.0
Related show

Commit Message

Laurent Pinchart July 7, 2023, 9:02 a.m. UTC 
The provider g_autoptr variable introduced by commit adb1bbb748a1
("tests: gstreamer: Test cameras' enumeration from GstDeviceProvider")
is left uninitialized when declared. The cleanup function could thus get
called on an unitialized variable if the scope was exited before the
variable gets initialized. This can't occur here, but gcc 8.4.0 still
complains about it:

/usr/include/glib-2.0/glib/gmacros.h: In member function ‘virtual int GstreamerDeviceProviderTest::run()’:
/usr/include/glib-2.0/glib/gmacros.h:1049:27: error: ‘provider’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
     { if (_ptr) (cleanup) ((ParentName *) _ptr); }                                                              \
                           ^
../test/gstreamer/gstreamer_device_provider_test.cpp:37:32: note: ‘provider’ was declared here
   g_autoptr(GstDeviceProvider) provider;

Silence the error by initializing the variable to NULL at declaration
time. This is a good practice in any case, as later refactoring could
otherwise introduce a scope exit before initialization.

Fixes: adb1bbb748a1 ("tests: gstreamer: Test cameras' enumeration from GstDeviceProvider")
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 test/gstreamer/gstreamer_device_provider_test.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: adb1bbb748a12eb2427f2d19a46fae55d99b623a

Comments

Umang Jain July 7, 2023, 9:03 a.m. UTC | #1 
Hi Laurent,

On 7/7/23 11:02 AM, Laurent Pinchart wrote:
> The provider g_autoptr variable introduced by commit adb1bbb748a1
> ("tests: gstreamer: Test cameras' enumeration from GstDeviceProvider")
> is left uninitialized when declared. The cleanup function could thus get
> called on an unitialized variable if the scope was exited before the
> variable gets initialized. This can't occur here, but gcc 8.4.0 still
> complains about it:
>
> /usr/include/glib-2.0/glib/gmacros.h: In member function ‘virtual int GstreamerDeviceProviderTest::run()’:
> /usr/include/glib-2.0/glib/gmacros.h:1049:27: error: ‘provider’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>       { if (_ptr) (cleanup) ((ParentName *) _ptr); }                                                              \
>                             ^
> ../test/gstreamer/gstreamer_device_provider_test.cpp:37:32: note: ‘provider’ was declared here
>     g_autoptr(GstDeviceProvider) provider;
>
> Silence the error by initializing the variable to NULL at declaration
> time. This is a good practice in any case, as later refactoring could
> otherwise introduce a scope exit before initialization.
>
> Fixes: adb1bbb748a1 ("tests: gstreamer: Test cameras' enumeration from GstDeviceProvider")
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

LGTM, ship it!

Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>


> ---
>   test/gstreamer/gstreamer_device_provider_test.cpp | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/test/gstreamer/gstreamer_device_provider_test.cpp b/test/gstreamer/gstreamer_device_provider_test.cpp
> index c8606b90559b..237af8cd934e 100644
> --- a/test/gstreamer/gstreamer_device_provider_test.cpp
> +++ b/test/gstreamer/gstreamer_device_provider_test.cpp
> @@ -34,7 +34,7 @@ protected:
>   
>   	int run() override
>   	{
> -		g_autoptr(GstDeviceProvider) provider;
> +		g_autoptr(GstDeviceProvider) provider = NULL;
>   		GList *devices, *l;
>   		std::vector<std::string> cameraNames;
>   		std::unique_ptr<libcamera::CameraManager> cm;
>
> base-commit: adb1bbb748a12eb2427f2d19a46fae55d99b623a
Kieran Bingham July 11, 2023, 2:56 p.m. UTC | #2 
Quoting Umang Jain via libcamera-devel (2023-07-07 10:03:45)
> Hi Laurent,
> 
> On 7/7/23 11:02 AM, Laurent Pinchart wrote:
> > The provider g_autoptr variable introduced by commit adb1bbb748a1
> > ("tests: gstreamer: Test cameras' enumeration from GstDeviceProvider")
> > is left uninitialized when declared. The cleanup function could thus get
> > called on an unitialized variable if the scope was exited before the
> > variable gets initialized. This can't occur here, but gcc 8.4.0 still
> > complains about it:
> >
> > /usr/include/glib-2.0/glib/gmacros.h: In member function ‘virtual int GstreamerDeviceProviderTest::run()’:
> > /usr/include/glib-2.0/glib/gmacros.h:1049:27: error: ‘provider’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >       { if (_ptr) (cleanup) ((ParentName *) _ptr); }                                                              \
> >                             ^
> > ../test/gstreamer/gstreamer_device_provider_test.cpp:37:32: note: ‘provider’ was declared here
> >     g_autoptr(GstDeviceProvider) provider;
> >
> > Silence the error by initializing the variable to NULL at declaration
> > time. This is a good practice in any case, as later refactoring could
> > otherwise introduce a scope exit before initialization.
> >
> > Fixes: adb1bbb748a1 ("tests: gstreamer: Test cameras' enumeration from GstDeviceProvider")
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> LGTM, ship it!
> 
> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Will pick for merge.

> 
> 
> > ---
> >   test/gstreamer/gstreamer_device_provider_test.cpp | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/test/gstreamer/gstreamer_device_provider_test.cpp b/test/gstreamer/gstreamer_device_provider_test.cpp
> > index c8606b90559b..237af8cd934e 100644
> > --- a/test/gstreamer/gstreamer_device_provider_test.cpp
> > +++ b/test/gstreamer/gstreamer_device_provider_test.cpp
> > @@ -34,7 +34,7 @@ protected:
> >   
> >       int run() override
> >       {
> > -             g_autoptr(GstDeviceProvider) provider;
> > +             g_autoptr(GstDeviceProvider) provider = NULL;
> >               GList *devices, *l;
> >               std::vector<std::string> cameraNames;
> >               std::unique_ptr<libcamera::CameraManager> cm;
> >
> > base-commit: adb1bbb748a12eb2427f2d19a46fae55d99b623a
>

Patch
diff mbox series 

diff --git a/test/gstreamer/gstreamer_device_provider_test.cpp b/test/gstreamer/gstreamer_device_provider_test.cpp
index c8606b90559b..237af8cd934e 100644
--- a/test/gstreamer/gstreamer_device_provider_test.cpp
+++ b/test/gstreamer/gstreamer_device_provider_test.cpp
@@ -34,7 +34,7 @@  protected:
 
 	int run() override
 	{
-		g_autoptr(GstDeviceProvider) provider;
+		g_autoptr(GstDeviceProvider) provider = NULL;
 		GList *devices, *l;
 		std::vector<std::string> cameraNames;
 		std::unique_ptr<libcamera::CameraManager> cm;