Closed Bug 1227481 Opened 8 years ago Closed 8 years ago

[Static Analysis][Result of operation is garbage or undefined] Function OpenCoreDebugFiles from media/webrtc/trunk/webrtc/modules/audio_processing/aec/aec_core.c


(Core :: WebRTC, defect)

Not set



Tracking Status
firefox45 --- fixed
backlog parking-lot


(Reporter: andi, Assigned: andi)


(Blocks 1 open bug)


(Keywords: clang-analyzer)


(1 file, 1 obsolete file)

The Static Analysis tool Scan-Build added warning that the left operand of > can be a garbage value in function OpenCoreDebugFiles where  aec->sampFreq is used without being initialized.
Attached patch Bug 1227481.diff (obsolete) — Splinter Review
Very low priority, and causes no problems - garbage values would merely cause either 16000 or random value between 0-15999 to be used as the wav frequency - and this would likely only be able to happen in an error path where no usable debug data would exist.  And if the file is corrupted, it's not a problem.

That said, ensuring the structure is safely allocated is worthwhile.  Though I'd use calloc perhaps instead; and clearing with "sizeof(FooStructure)" is error-prone, such as if the malloc is changed to sizeof(FooBarStructure) or sizeof(FooStructure+N).  memset(x, 0, sizeof(*x)) is safer.

Also: upstream issue; file a bug at the issue tracker.
backlog: --- → parking-lot
Comment on attachment 8691339 [details] [diff] [review]
Bug 1227481.diff

Review of attachment 8691339 [details] [diff] [review]:

r+ with tweak (sizeof(*aec))
Attachment #8691339 - Flags: review+
Attached patch Bug 1227481.diffSplinter Review
I definitely agree with you, best practice is to use sizeof(*data) in cases when you alloc more memory than the actual size of the type that data points to.
Attachment #8691339 - Attachment is obsolete: true
Keywords: checkin-needed
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
No longer blocks: webrtc_upstream_bugs
You need to log in before you can comment on or make changes to this bug.