Closed
Bug 334306
Opened 18 years ago
Closed 16 years ago
useless null check in nsDestroyJSPrincipals
Categories
(Core :: Security: CAPS, defect)
Core
Security: CAPS
Tracking
()
RESOLVED
FIXED
People
(Reporter: timeless, Assigned: timeless)
References
(Blocks 1 open bug, )
Details
(Keywords: coverity)
Attachments
(1 file)
634 bytes,
patch
|
dbaron
:
review+
dveditz
:
superreview+
mtschrep
:
approval1.9+
|
Details | Diff | Splinter Review |
Attachment #218633 -
Flags: superreview?(dveditz)
Attachment #218633 -
Flags: review?(dbaron)
Comment 2•18 years ago
|
||
The null check was added to stop a free memory write (bug 52382) -- are we sure it's no longer needed? Coverity is simply complaining about the inconsistency, it's possible it's the unchecked AddRef that's wrong. Then again if that's true then ::Subsumes and probably other places are equally dangerous.
i'm definitely not sure. hence the review requests to the relevant parties.
Comment 4•18 years ago
|
||
(In reply to comment #2) > The null check was added to stop a free memory write (bug 52382) -- are we sure > it's no longer needed? That's not where the null-check was added -- that's just where we changed from hiding the null-check within NS_IF_RELEASE since the null-assignment (after the release) within NS_IF_RELEASE was causing a free memory write.
Comment 5•18 years ago
|
||
It's been null-checked, perhaps reflexively, since the beginning (revision 1.1.2.1).
Comment 6•18 years ago
|
||
Comment on attachment 218633 [details] [diff] [review] drop null check I'd be slightly happier if nsJSPrincipals::Init() returned an error (or even asserted) when passed a null aPrincipal, but we must not ever do so or we'd be crashing. sr=dveditz
Attachment #218633 -
Flags: superreview?(dveditz) → superreview+
Comment 7•17 years ago
|
||
Comment on attachment 218633 [details] [diff] [review] drop null check r=dbaron
Attachment #218633 -
Flags: review?(dbaron) → review+
Updated•16 years ago
|
Attachment #218633 -
Flags: approval1.9+
Updated•16 years ago
|
Keywords: checkin-needed
Flags: blocking1.9? → blocking1.9-
Updated•16 years ago
|
Keywords: checkin-needed
OS: Mac OS X → All
QA Contact: caps
Hardware: Macintosh → All
Whiteboard: [needs landing]
mozilla/caps/src/nsJSPrincipals.cpp 1.27
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Summary: hopefully useless null check in nsDestroyJSPrincipals → useless null check in nsDestroyJSPrincipals
Whiteboard: [needs landing]
Updated•6 years ago
|
Blocks: coverity-analysis
You need to log in
before you can comment on or make changes to this bug.
Description
•