189839 - mozilla -remote "ping()" gives wrong return value

Status: VERIFIED FIXED

(Core Graveyard :: X-remote, defect)

Description

[Robert Kaiser]

In the last few days, the ping x-remote command regressed (I'm currently using a
trunk cvs build of 2003-01-19-07).

mozilla -remote "ping()" should give a return code of 0 when a running Mozilla
instance is found, and 1 if there's no running instance detected.
Currently it always returns 1 though.

Simple testcase:
cd to your Mozilla directory and type the following into your command line:
if `./mozilla -remote "ping()"`; then echo 'running'; else echo 'not running';fi

other possibilty:
./mozilla -remote "ping()"; echo $?
(prints out the return value)

this is what I get currently if Mozilla is running:
robert@robert:~> /opt/mozilla-build/mozilla -remote "ping()"; echo $?
1
(should return 0)

if Mozilla is not running:
robert@robert:~> /opt/mozilla-build/mozilla -remote "ping()"; echo $?
No running window found.
1
(correct return value of 1 - shouldn't it be completely silent though?)

It seems as if it's detecting the running instance and just retuning a false
value then...

Comment 1

[Robert Kaiser]

Just to be sure it's a regression:

robert@robert:~> /opt/mozilla1.2.1/mozilla/mozilla -remote "ping()"; echo $?
0
(Mozilla is running)

and
robert@robert:~> /opt/mozilla1.2.1/mozilla/mozilla -remote "ping()"; echo $?
No running window found.
2


OK, so the correct return code would be 2, which means we have an error in both
the running and not-running case.

biesi could also see this, so it's not only my build...

Comment 2

[Doug Turner (:dougt)]

this should work for 1.3b.  this bug is probably a dup of the same problem which
causes 189591.

Comment 3

[Robert Kaiser]

This is what caused that problem:

http://lxr.mozilla.org/mozilla/source/xpfe/bootstrap/nsAppRunner.cpp#1918

has the following lines now:
1918   if (argused) {
1919 #ifdef XPCOM_GLUE
1920     GRE_Shutdown();
1921 #else
1922     NS_ShutdownXPCOM(nsnull);
1923 #endif
1924   }

previously we had that:
  if (argused) {
    if (NS_SUCCEEDED(rv)) // only call NS_ShutdownXPCOM if Init succeeded.
      NS_ShutdownXPCOM(nsnull);
    return remoterv;
  }

so the "return remoterv;" line was removed even though it should be there...

Comment 4

[Doug Turner (:dougt)]

ouch... good catch.

Blizzard, does this look good to you?

Index: nsAppRunner.cpp
===================================================================
RCS file: /cvsroot/mozilla/xpfe/bootstrap/nsAppRunner.cpp,v
retrieving revision 1.383
diff -u -r1.383 nsAppRunner.cpp
--- nsAppRunner.cpp	17 Jan 2003 03:08:36 -0000	1.383
+++ nsAppRunner.cpp	20 Jan 2003 23:58:19 -0000
@@ -1921,6 +1921,7 @@
 #else
     NS_ShutdownXPCOM(nsnull);
 #endif
+    return remoterv;
   }
 #endif
 

Comment 5

[Robert Kaiser]

Attachment: putting the return back in

This is what I'd consider to be the patch. A one-liner - just put back in the
return line...

Comment 6

[Robert Kaiser]

A little more context for the diff would look better, I guess...

Well, I won't do a new attachment, I'll just paste it inline here - I just to
see it's in the right place:

   if (argused) {
 #ifdef XPCOM_GLUE
     GRE_Shutdown();
 #else
     NS_ShutdownXPCOM(nsnull);
 #endif
+    return remoterv;
   }
 #endif

   nsresult mainResult = main1(argc, argv, nativeApp ? (nsISupports*)nativeApp :
(nsISupports*)splash);

Comment 7

[Robert Kaiser]

(dougt: oops, btw, didn't see you posting the same in the same minute where I
attached the patch... I was so happy I found a solution for the problem that I
didn't look at that :)

Comment 8

[Doug Turner (:dougt)]

Checking in nsAppRunner.cpp;
/cvsroot/mozilla/xpfe/bootstrap/nsAppRunner.cpp,v  <--  nsAppRunner.cpp
new revision: 1.384; previous revision: 1.383
done


I just checked in the fix.  Can you please verify that this does work for you?

Comment 9

[Doug Turner (:dougt)]

kario, thank you very much for finding the problem!

Comment 10

[Robert Kaiser]

Comment on attachment 112108 [details] [diff] [review]
putting the return back in

no problem.. digging into that code for such likely-to-be-simple bugs is fun,
and also helps me understanding Mozilla's C++ source a bit better... :)

Comment 11

[Robert Kaiser]

Yes, it's working again with that patch. verified.