Closed
Bug 333674
Opened 19 years ago
Closed 19 years ago
We leak oldVal on OOM in nsUint32ToContentHashEntry::PutContent
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
People
(Reporter: jwatt, Assigned: jwatt)
References
(Blocks 1 open bug, )
Details
(Keywords: coverity, memory-leak)
Attachments
(1 file, 1 obsolete file)
|
(deleted),
patch
|
bzbarsky
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
The call to GetContent could return null, in which case NS_RELEASE dereference null.
|
Assignee | |
Comment 1•19 years ago
|
||
Attachment #218127 -
Flags: superreview?(roc)
Attachment #218127 -
Flags: review?(roc)
|
|
||
Comment 2•19 years ago
|
||
> The call to GetContent could return null
Not really. The code is:
186 if (GetContent()) {
187 nsIContent* oldVal = GetContent();
Now perhaps we want to only call GetContent() once and then just test oldVal? Would probably make coverity happier and mean only two conditional tests there instead of the current 3....
I'd really have expected coverity to not flag this, though. Is there a way to report to them that this is bogus? Does flagging the report as "FALSE" in scan.coverity.com do that?
|
|
||
Updated•19 years ago
|
Attachment #218127 -
Flags: superreview?(roc)
Attachment #218127 -
Flags: superreview-
Attachment #218127 -
Flags: review?(roc)
Attachment #218127 -
Flags: review-
|
|
Assignee | |
Comment 3•19 years ago
|
||
True, I should have noticed that. :-/ Nevertheless, there's still a leak on OOM that we should fix.
Attachment #218127 -
Attachment is obsolete: true
Attachment #218133 -
Flags: superreview?(roc)
Attachment #218133 -
Flags: review?(roc)
|
|
Assignee | |
Comment 4•19 years ago
|
||
I don't know if there's a special way to report false positives to coverity. I'd hope they keep looking at the reports marked FALSE though to improve their code.
|
|
||
Comment 5•19 years ago
|
||
Comment on attachment 218133 [details] [diff] [review]
patch for leak
SetContent addrefs, so this will double-addref the content and leak.
Attachment #218133 -
Flags: superreview?(roc)
Attachment #218133 -
Flags: superreview-
Attachment #218133 -
Flags: review?(roc)
Attachment #218133 -
Flags: review-
|
|
||
Comment 6•19 years ago
|
||
Comment on attachment 218133 [details] [diff] [review]
patch for leak
Never mind me. I need to learn to read.
Attachment #218133 -
Flags: superreview-
Attachment #218133 -
Flags: superreview+
Attachment #218133 -
Flags: review-
Attachment #218133 -
Flags: review+
fwiw they do read through FALSEs, but if there's a specific pattern we want them to learn, we're supposed to email something.
|
|
Assignee | |
Comment 8•19 years ago
|
||
Checked in.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Summary: Leak oldVal on OOM in nsUint32ToContentHashEntry::PutContent → We leak oldVal on OOM in nsUint32ToContentHashEntry::PutContent
|
||
Updated•6 years ago
|
Blocks: coverity-analysis
|
||
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•