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 750567 - rtpvp8depay: FTBFS because of access beyond end of array compiler warning
rtpvp8depay: FTBFS because of access beyond end of array compiler warning
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
1.5.1
Other Linux
: Normal normal
: 1.5.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 751822 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2015-06-08 14:30 UTC by Chris
Modified: 2015-07-02 07:52 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to fix FTBFS of gst-plugins-good-1.5.1 (579 bytes, patch)
2015-06-08 14:30 UTC, Chris
none Details | Review
alternative patch (945 bytes, patch)
2015-06-08 15:12 UTC, Luis de Bethencourt
none Details | Review
Patch with commit message (1.20 KB, patch)
2015-06-08 18:59 UTC, Chris
committed Details | Review

Description Chris 2015-06-08 14:30:51 UTC
Created attachment 304772 [details] [review]
Patch to fix FTBFS of gst-plugins-good-1.5.1

plugins-good FTBFS with the latest gcc-4.9 snapshot because of a possible access beyond the end of an array. Patch to fix is attached.
Comment 1 Luis de Bethencourt 2015-06-08 15:09:29 UTC
Hello Chris,

Your patch changes behaviour. You removed the check for if offset >= self->partition_offset[i].

How about my proposed patch. Does it work for you? Which version of gcc-4.9 are you using?

I have 4.9.1 and I am not seeing this FTBFS.
Comment 2 Luis de Bethencourt 2015-06-08 15:12:01 UTC
Created attachment 304782 [details] [review]
alternative patch
Comment 3 Chris 2015-06-08 16:11:51 UTC
I don't think it does change behaviour, at least not in a meaningful way. The logic of the present code implies that each value in the array has a value greater than its predecessor. The current code assumes that the value of offset is sane. The code after my patch is applied makes the same assumption. My code is simpler. If offset is less than self->partition_offset[1], then 0 must be the value to return, otherwise, if offset is less than self->partition_offset[2], then 1 must be the value to return and so on. Consequently, the >= test seems unnecessary. The only behaviour change that I can see is what will be returned if offset is not a sane value. Perhaps I'm misinterpreting the current code. I haven't looked at the code that loads the values into the array.

My gcc is the latest snapshot  4.9.3 20150603 (prerelease).
Comment 4 Luis de Bethencourt 2015-06-08 16:19:58 UTC
I agree with your explanation. Specially after testing both fixes.

If you could upload a new version of your patch with a commit message. I am happy to push it.

Just in case:
git add gst/rtp/gstrtpvp8pay.c
git commit
git format-patch -o /tmp/ HEAD~1
attach here the 0001* patch file dropped in /tmp/

Thanks!
Comment 5 Chris 2015-06-08 18:59:46 UTC
Created attachment 304804 [details] [review]
Patch with commit message
Comment 6 Luis de Bethencourt 2015-06-08 19:24:49 UTC
Review of attachment 304804 [details] [review]:

Thanks Chris! Merged :)

Notice the commit message format for future patch submissions.

commit e29f231e5d032fea63a55ba0258a4ab3e2b13f67
Author: Chris Clayton <chris2553@googlemail.com>
Date:   Mon Jun 8 19:42:30 2015 +0100

    rtpvp8depay: potential access beyond end of array

    Compiling (with gcc-4.9-20150603) produces an error because of an access beyond
    the end of an array. This patch fixes the error by initializing the loop
    control/array index variable (i) to 1 and returning i - 1 when a match is found.
    Also, because the values stored in the array increase in value as the index
    increases, the >= test unnecessary, so it is removed.
Comment 7 Sebastian Dröge (slomo) 2015-07-02 07:52:06 UTC
*** Bug 751822 has been marked as a duplicate of this bug. ***