Last Comment Bug 776661 - silk_get_TOC doesn't zero out the entire silk_TOC_struct
: silk_get_TOC doesn't zero out the entire silk_TOC_struct
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: mozilla17
Assigned To: :Ehsan Akhgari
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-23 13:03 PDT by :Ehsan Akhgari
Modified: 2012-08-14 12:22 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (v1) (974 bytes, patch)
2012-07-23 13:05 PDT, :Ehsan Akhgari
giles: review+
Details | Diff | Splinter Review
Patch (v2) (2.79 KB, patch)
2012-07-23 14:28 PDT, Ralph Giles (:rillian) needinfo me
giles: review+
Details | Diff | Splinter Review

Description :Ehsan Akhgari 2012-07-23 13:03:32 PDT
/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.
Comment 1 :Ehsan Akhgari 2012-07-23 13:05:40 PDT
Created attachment 645035 [details] [diff] [review]
Patch (v1)
Comment 2 Ralph Giles (:rillian) needinfo me 2012-07-23 14:17:24 PDT
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 3 Ralph Giles (:rillian) needinfo me 2012-07-23 14:18:36 PDT
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.
Comment 4 Ralph Giles (:rillian) needinfo me 2012-07-23 14:28:52 PDT
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+.
Comment 5 :Ehsan Akhgari 2012-07-23 14:40:55 PDT
I'll check this in shortly.  Thanks Ralph!
Comment 7 Ed Morley [:emorley] 2012-07-24 02:58:50 PDT
https://hg.mozilla.org/mozilla-central/rev/8020aa862817
Comment 8 Timothy B. Terriberry (:derf) 2012-08-14 12:22:14 PDT
This was committed upstream as http://git.xiph.org/?p=opus.git;a=commit;h=622046c1f1230e107664e35873d46b731f32df41

Note You need to log in before you can comment on or make changes to this bug.