334549 - Fix for Coverty CID 179

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Jon Smirl]

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/

Comment 1

[Jon Smirl]

Attachment: Fix for CID 179

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.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Can you attach a cvs diff -up8 (and -w if it helps)?

Comment 3

[Jon Smirl]

Attachment: same change, diff easier to read

Comment 4

[Daniel Veditz [:dveditz]]

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.

Comment 5

[Daniel Veditz [:dveditz]]

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 6

[Jon Smirl]

Attachment: cvs diff -up8, previous patch was cvs diff -up8 -w

Comment 7

[Benjamin Smedberg]

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.

Comment 8

[Nickolay_Ponomarev]

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).

Comment 9

[Andrew Schultz]

/cvsroot/mozilla/xpcom/build/nsXPComInit.cpp,v  <--  nsXPComInit.cpp
new revision: 1.244; previous revision: 1.243