[FIXED JENKINS-9634] fixed a bug in the Mac OS X arg parsing code

jenkinsci · Nov 8, 2011 · 5506780 · 5506780
1 parent 737a7df
commit 5506780
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 7 deletions.
diff --git a/changelog.html b/changelog.html
@@ -64,6 +64,9 @@
   <li class=bug>
     Debian init script now returns the proper exit code from the 'status' command.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-11306">issue 11306</a>)
+  <li class=bug>
+    Fixed a bug in Mac OS X ProcessKiller argument parsing.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-9634">issue 9634</a>)
   <li class=rfe>
     Sortable table wasn't "stable" when there are same values in different rows
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-11551">issue 11551</a>)

diff --git a/core/src/main/java/hudson/util/ProcessTree.java b/core/src/main/java/hudson/util/ProcessTree.java
@@ -1001,7 +1001,7 @@ String readString() {
                         }
 
                         void skip0() {
-                            // skip trailing '\0's
+                            // skip padding '\0's
                             while(getByte(offset)=='\0')
                                 offset++;
                         }
@@ -1052,20 +1052,22 @@ void skip0() {
                         * \---------------/ 0xffffffff
                         */
 
-                    int nargs = m.readInt();
+                    // I find the Darwin source code of the 'ps' command helpful in understanding how it does this:
+                    // see http://www.opensource.apple.com/source/adv_cmds/adv_cmds-147/ps/print.c
+                    int argc = m.readInt();
                     String args0 = m.readString(); // exec path
+                    m.skip0();
                     try {
-                        for( int i=0; i<nargs; i++) {
-                            m.skip0();
+                        for( int i=0; i<argc; i++) {
                             arguments.add(m.readString());
                         }
                     } catch (IndexOutOfBoundsException e) {
-                        throw new IllegalStateException("Failed to parse arguments: arg0="+args0+", arguments="+arguments+", nargs="+nargs,e);
+                        throw new IllegalStateException("Failed to parse arguments: arg0="+args0+", arguments="+arguments+", nargs="+argc,e);
                     }
 
-                    // this is how you can read environment variables
+                    // read env vars that follow
                     while(m.peek()!=0)
-                    envVars.addLine(m.readString());
+                        envVars.addLine(m.readString());
                 } catch (IOException e) {
                     // this happens with insufficient permissions, so just ignore the problem.
                 }