41996 - nsCmdLineService::GetCmdLineValue needs to clone aResult

Status: VERIFIED FIXED

(Core Graveyard :: Cmd-line Features, defect, P3)

Description

[Simon Fraser [no longer active]]

nsCmdLineService::GetCmdLineValue returns a char*, but, since its an XPCOM 
interface, it should clone the result.

Currently, the call to this function in LaunchApplicationWithArgs():

  nsXPIDLCString cmdResult;
  rv = cmdLineArgs->GetCmdLineValue(commandLineArg, getter_Copies(cmdResult));

is causing a double deletion.

However, there are a bunch of places that call GetCmdLineValue() without deleting 
the result. These need to be fixed.

Comment 1

[Simon Fraser [no longer active]]

cc bruce

Comment 2

[Simon Fraser [no longer active]]

BTW, this double deletion is causing various crashes on quit for me.

Comment 3

[Alec Flett]

ohh!! I've been trying to figure this out all evening. Thanks for tracking this 
down simon.. I'm getting lots of assertions/crashes as well.

Nominate for beta2 - this makes the app crash on exit

Comment 4

[Simon Fraser [no longer active]]

alecf: can I give this to you? You should fix callers of GetCmdLineValue who 
don't delete the string to do so, after fixing the bug itself.

Comment 5

[Alec Flett]

sure :)
-The Mozilla Janitor

Comment 6

[Alec Flett]

Attaching the patch.

Adding crash keyword because I'm realizing this can cause random crashes 
throughout the app as well.

Adding the dogfood keyword because this has been preventing me from running 
purify on mail. (because purify will not report results if the app crashes)

Comment 7

[Alec Flett]

Attachment: patch to fix all consumers of GetCmdLineValue

Comment 8

[Simon Fraser [no longer active]]

this may be causing bug 41339 too. alecf, can you check into M16 too?

Comment 9

[leger]

Putting on [dogfood+][nsbeta2-] radar.

Comment 10

[Alec Flett]

fix is in.

Comment 11

[Alec Flett]

fix checked into the branch as well.

Comment 12

[sairuh (rarely reading bugmail)]

i hereby apply rubberstamp verification. :-) spoke w/simon --this is a codelevel
fix that's tricky to verify for an enduser (dunno why this was labelled dogfood,
so removing that kw).