Closed Bug 334549 Opened 14 years ago Closed 14 years ago

Fix for Coverty CID 179

Categories

(Core :: XPCOM, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: jonsmirl, Assigned: jonsmirl)

References

(Blocks 1 open bug, )

Details

(Keywords: coverity)

Attachments

(2 files, 1 obsolete file)

Event assign_zero: Variable "compMgr" assigned value 0.
Event var_deref_model: Variable "compMgr" tracked as NULL was passed to a function that dereferences it.

See the Coverity bugs at 
http://scan.coverity.com:7454/
Attached patch Fix for CID 179 (obsolete) — Splinter Review
It may be possible to rearrange this better but I'm not sure if it is ok to stomp nsComponentManagerImpl::gComponentManager when xpcom is initializaed.
Can you attach a cvs diff -up8 (and -w if it helps)?
Keywords: coverity
Attachment #218894 - Attachment is obsolete: true
Better to have public reference links, adding bonsai link to URL field.
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/xpcom/build/nsXPComInit.cpp&rev=1.243&mark=517-519,573#516

The "bug" Coverity found assumes NS_InitXPCOM* will be called multiple times (doing so looks like a bad idea, but there's no explicit check against it). The second time through gComponentManager will be set, compMgr remains null, and then it's dereferenced down on line 573.

Some of the early returns in the "if (nsComponentManagerImpl::gComponentManager == NULL)" block leak the newly created component manager.
Attachment #218915 - Flags: review?(benjamin)
Comment 4 was a description of the original problem, I mid-aired with the new patch and hadn't bothered to puzzle out what the first patch was doing. Looks like attachment 218915 [details] [diff] [review] fixes all the stuff I mentioned.

A diff without the -w option would be good if you need someone else to check this in for you. (-w is great for reviews, not for checking in.)
Comment on attachment 218915 [details] [diff] [review]
same change, diff easier to read

It's not a real bug but we might as well "fix" it.
Attachment #218915 - Flags: superreview?(darin)
Attachment #218915 - Flags: review?(benjamin)
Attachment #218915 - Flags: review+
Attachment #218915 - Flags: superreview?(darin) → superreview+
Jon, if you want the patch to be checked in, you should ask someone to do it or add [checkin needed] to the whiteboard field of the bug (the first way is usually more effective).
Assignee: nobody → jonsmirl
Whiteboard: checkin needed
/cvsroot/mozilla/xpcom/build/nsXPComInit.cpp,v  <--  nsXPComInit.cpp
new revision: 1.244; previous revision: 1.243
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: checkin needed
You need to log in before you can comment on or make changes to this bug.