Re: java.lang.Process.exitValue() wrong

[Bryce McKinlay <bryce at waitaki dot otago dot ac dot nz>]

Weiqi Gao wrote:

>>I believe the system call wait() (which presumably Process.waitFor()  is
>>using returns an encoded value containing the process return value and how
>>the process was terminated.  Read the man page for wait()  for details.
>>
>
>Then the implementation of java.lang.Process should probably use the
>appropriate macros to extract the true status and return that from
>waitFor() and exitValue().
>

Yep. Here's a patch, which I am checking in.

regards

Bryce.


2001-09-24  Bryce McKinlay  <bryce@waitaki.otago.ac.nz>

	* java/lang/PosixProcess.java (exitValue): Implement here. Throw 
	IllegalThreadStateException if process hasn't exited yet.
	* java/lang/natPosixProcess.cc (exitValue): Removed.
	(waitFor): Only check thread interrupted status if waitpid() returned
	an error. Use WIFEXITED and WEXITSTATUS to process process's exit
	value.

Index: java/lang/PosixProcess.java
===================================================================
RCS file: /cvs/gcc/egcs/libjava/java/lang/PosixProcess.java,v
retrieving revision 1.3
diff -u -r1.3 PosixProcess.java
--- PosixProcess.java	2000/03/07 19:55:26	1.3
+++ PosixProcess.java	2001/09/24 04:40:58
@@ -26,7 +26,13 @@
 final class ConcreteProcess extends Process
 {
   public native void destroy ();
-  public native int exitValue ();
+
+  public int exitValue ()
+  {
+    if (! hasExited)
+      throw new IllegalThreadStateException("Process has not exited");
+    return status;
+  }
 
   public InputStream getErrorStream ()
   {
Index: java/lang/natPosixProcess.cc
===================================================================
RCS file: /cvs/gcc/egcs/libjava/java/lang/natPosixProcess.cc,v
retrieving revision 1.8
diff -u -r1.8 natPosixProcess.cc
--- natPosixProcess.cc	2001/03/26 07:05:32	1.8
+++ natPosixProcess.cc	2001/09/24 04:40:58
@@ -49,27 +49,6 @@
 }
 
 jint
-java::lang::ConcreteProcess::exitValue (void)
-{
-  if (! hasExited)
-    {
-      int wstat;
-      pid_t r = waitpid ((pid_t) pid, &wstat, WNOHANG);
-      if (r == -1)
-	{
-	  jstring x = JvNewStringLatin1 (strerror (errno));
-	  throw new IllegalThreadStateException (x);
-	}
-
-      hasExited = true;
-      // Just use the raw status.  FIXME: what is right?
-      status = wstat;
-    }
-
-  return status;
-}
-
-jint
 java::lang::ConcreteProcess::waitFor (void)
 {
   if (! hasExited)
@@ -77,15 +56,21 @@
       int wstat;
       int r = waitpid ((pid_t) pid, &wstat, 0);
 
-      if (r != -1)
+      if (r == -1)
+        {
+	  if (java::lang::Thread::interrupted())
+	    throw new InterruptedException (JvNewStringLatin1 (strerror
+	      (errno)));
+	}
+      else
 	{
 	  hasExited = true;
-	  // Just use the raw status.  FIXME: what is right?
-	  status = wstat;
-	}
 
-      if (java::lang::Thread::interrupted())
-	throw new InterruptedException (JvNewStringLatin1 ("wait interrupted"));
+	  if (WIFEXITED (wstat))
+	    status = WEXITSTATUS (wstat);
+	  else
+	    status = -1;
+	}
     }
 
   return status;