673154 - possible memory leak in WebM reader

Status: RESOLVED FIXED

Description

[David Volgyes]

Created attachment 547436 [details] [diff] [review]
patch with my proposed solution

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.

Comment 1

[Chris Pearce (:cpearce)]

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.

Comment 2

[Matthew Gregan [:kinetik]]

Created attachment 549287 [details] [diff] [review]
patch v0

Comment 3

[Matthew Gregan [:kinetik]]

Comment on attachment 549287 [details] [diff] [review]
patch v0

Oops, this is missing a buffer.forget() in once place.

Comment 4

[Matthew Gregan [:kinetik]]

Created attachment 549288 [details] [diff] [review]
patch v1

Comment 5

[Chris Pearce (:cpearce)]

Comment on attachment 549288 [details] [diff] [review]
patch v1

Review of attachment 549288 [details] [diff] [review]:
-----------------------------------------------------------------

Comment 6

[:Ehsan Akhgari (needinfo please, extremely long backlog)]

http://hg.mozilla.org/mozilla-central/rev/a7260d755392