silk_get_TOC doesn't zero out the entire silk_TOC_struct

RESOLVED FIXED in mozilla17

Status

()

Core
Audio/Video
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

Trunk
mozilla17
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

/media/storage/moz/mozilla-inbound/media/libopus/silk/dec_API.c:360:39: warning: 'silk_memset' call operates on objects of type 'silk_TOC_struct' while the size is based on a
      different type 'silk_TOC_struct *' [-Wsizeof-pointer-memaccess]
    silk_memset( Silk_TOC, 0, sizeof( Silk_TOC ) );
                 ~~~~~~~~             ^~~~~~~~
/media/storage/moz/mozilla-inbound/media/libopus/silk/dec_API.c:360:39: note: did you mean to dereference the argument to 'sizeof' (and multiply it by the number of elements)?
    silk_memset( Silk_TOC, 0, sizeof( Silk_TOC ) );
                                      ^~~~~~~~
1 warning generated.
Created attachment 645035 [details] [diff] [review]
Patch (v1)
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #645035 - Flags: review?(giles)
Analysis by :derf from #developers on irc:

This doesn't affect our code; silk_get_TOC() is never called; it's left over code used by some other consumers of the SILK codec outside the opus tree.

If it were called, it's probably low risk, since it would zero at least the first field, and the other fields are initialized explicitly, assuming the caller uses nFramesPerPayload consistently.
Comment on attachment 645035 [details] [diff] [review]
Patch (v1)

Fine with me, but please add it as an external patch file in the libopus directory and change update.sh to apply it.
Attachment #645035 - Flags: review?(giles) → review+
Created attachment 645075 [details] [diff] [review]
Patch (v2)

Updated to patch to conform to local style for maintaining differences from upstream. Carrying forward my own r+.
Attachment #645035 - Attachment is obsolete: true
Attachment #645075 - Flags: review+
Keywords: checkin-needed
I'll check this in shortly.  Thanks Ralph!
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/8020aa862817
Target Milestone: --- → mozilla17

Comment 7

5 years ago
https://hg.mozilla.org/mozilla-central/rev/8020aa862817
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
This was committed upstream as http://git.xiph.org/?p=opus.git;a=commit;h=622046c1f1230e107664e35873d46b731f32df41
You need to log in before you can comment on or make changes to this bug.