After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 728987 - qtdemux: 'caps' may be used uninitialized in this function.
qtdemux: 'caps' may be used uninitialized in this function.
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other All
: Normal normal
: 1.3.1
Assigned To: Luis de Bethencourt
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-04-25 21:53 UTC by Luis de Bethencourt
Modified: 2014-05-01 20:19 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fix (890 bytes, patch)
2014-04-25 21:56 UTC, Luis de Bethencourt
committed Details | Review
alternative fix (844 bytes, patch)
2014-04-25 22:07 UTC, Luis de Bethencourt
reviewed Details | Review

Description Luis de Bethencourt 2014-04-25 21:53:46 UTC
When building with -Werror=maybe-uninitialized we get:

qtdemux.c: In function 'qtdemux_parse_tree':
qtdemux.c:7480:18: error: 'caps' may be used uninitialized in this function [-Werror=maybe-uninitialized]
     stream->caps =
                  ^
qtdemux.c:10459:12: note: 'caps' was declared here
   GstCaps *caps;
            ^


Because in qtdemux_video_caps not all cases set the caps pointer to a value and the pointer isn't initialized at declaration.

Please confirm so I can push.
Comment 1 Luis de Bethencourt 2014-04-25 21:56:30 UTC
Created attachment 275174 [details] [review]
fix

Please confirm
Comment 2 Luis de Bethencourt 2014-04-25 21:58:31 UTC
A similar thing happens in qtdemux_audio_caps () and qtdemux_sub_caps () but in those functions all cases initialize the caps pointer.
Comment 3 Thiago Sousa Santos 2014-04-25 22:06:55 UTC
Review of attachment 275174 [details] [review]:

If the video fourcc is 'raw ' and the bpp is not one of the expected values it will indeed end up with an uninitialized caps pointer.

Nice catch!
Comment 4 Luis de Bethencourt 2014-04-25 22:07:54 UTC
Created attachment 275176 [details] [review]
alternative fix

The alternative is to initalize the caps in the only case that doesn't already do it.
Comment 5 Thiago Sousa Santos 2014-04-25 22:13:40 UTC
Review of attachment 275176 [details] [review]:

I still prefer the first patch as this one will create a video/x-raw stream without a specification about its format.

::: gst/isomp4/qtdemux.c
@@ +10524,3 @@
           break;
       }
+      caps = gst_caps_new_empty_simple ("video/x-raw");

You actually want to have this right after the /* unknown */ comment.
Comment 6 Luis de Bethencourt 2014-04-25 22:17:00 UTC
True! About the position of the gst_caps_new_empty_simple() but not important because I also prefer the first one.

Pushing. Thanks for the review :)
Comment 7 Thiago Sousa Santos 2014-04-25 22:28:10 UTC
Reopening so we get another patch to check for a NULL return from the qtdemux_video_caps function.
Comment 8 Luis de Bethencourt 2014-04-26 03:12:12 UTC
Will have a patch tomorrow :)
Comment 9 Luis de Bethencourt 2014-04-27 01:06:41 UTC
Pushed check for return of qtdemux_video_caps ().

http://cgit.freedesktop.org/gstreamer/gst-plugins-good/commit/?id=5dc2e6bef1cb35e9473dcb785d3711e0892c7e38

I will let you confirm and set to fixed/resolved.