Closed Bug 673154 Opened 10 years ago Closed 10 years ago

possible memory leak in WebM reader

Categories

(Core :: Audio/Video, defect)

Product:
Core
Component:
Audio/Video
Platform:
All
Other
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla8

People

(Reporter: david.volgyes, Assigned: kinetik)

References

Details

Attachments

(1 file, 2 obsolete files)

patch v1
4.05 KB, patch
cpearce
: review+
Details | Diff | Splinter Review
Attached patch patch with my proposed solution (obsolete) — — Splinter Review 
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:5.0) Gecko/20100101 Firefox/5.0
Build ID: 20110622232440

Steps to reproduce:

I tested the code with cppcheck 1.49,
and I checked some warning from the result.



Actual results:

Cppcheck showed several warnings, but some of them false positive.
This seems a real bug:
content/media/webm/nsWebMReader.cpp:498]: (error) Memory leak: buffer

The code allocates memory for temporary storage.
However, it does not release the memory when error happens.




Expected results:

- Memory should be released before "return". 
(A possible fix is attached, but please, review it before use.)
- Mozilla should use cppcheck for automated testing.
Component: General → Video/Audio
Product: Firefox → Core
QA Contact: general → video.audio
Nice catch! Please make |buffer| an nsAutoArrayPtr<SoundDataValue>, so that it's automatically released on scope exit instead. You'll need to call buffer.forget() when you pass it to the SoundData constructor, as that takes over ownership.
Attached patch patch v0 (obsolete) — — Splinter Review
Assignee: nobody → kinetik
Attachment #547436 - Attachment is obsolete: true
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #549287 - Flags: review?(chris)
Comment on attachment 549287 [details] [diff] [review]
patch v0

Oops, this is missing a buffer.forget() in once place.
Attachment #549287 - Flags: review?(chris)
Attached patch patch v1 — — Splinter Review
Attachment #549287 - Attachment is obsolete: true
Attachment #549288 - Flags: review?(chris)
Comment on attachment 549288 [details] [diff] [review]
patch v1

Review of attachment 549288 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #549288 - Flags: review?(chris) → review+
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/a7260d755392
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla8
Blocks: cppcheck
You need to log in before you can comment on or make changes to this bug.