Re: sdiff "subsidiary program `%s' failed" [diffutils 2.8.4-r1]

[Paul Eggert]

> From: "D. Wollmann" <address@hidden>
> Date: Sat, 11 Jan 2003 01:28:57 -0600

> distribution: diffutils 2.8.4-r1
> platform: Gentoo GNU/Linux 1.4_rc1
> 
> sdiff always prints an error message: "subsidiary program `?' failed"
> (where ? is the editor pathname set in the EDITOR shell variable) and returns 
> non-zero exit status.
> 
> At sdiff.c:704 there is a call to waitpid after calling pclose on a diff 
> process forked with popen, which probably returns ECHILD.

Sorry, I don't follow this.  At sdiff.c:704 there is a call to
waitpid, but if that code is executed then neither popen nor pclose
can possibly have been executed.  This is because waitpid is called
only if HAVE_WORKING_FORK || HAVE_WORKING_VFORK, whereas popen and
pclose are called only if ! (HAVE_WORKING_FORK || HAVE_WORKING_VFORK).

On Gentoo Linux, I would expect both HAVE_WORKING_FORK and
HAVE_WORKING_VFORK to be 1.  Is that true in your config.h?


> The following patch "works here", but as I am not a real C hacker, it should 
> be examined more carefully before use.

That patch will cause sdiff to not wait for its subprocess or
check its status.  That can't be right.

How about the following patch instead?

--- sdiff.c     2002/06/17 05:55:42     1.39
+++ sdiff.c     2002/09/05 21:06:55
@@ -263,15 +263,23 @@ perror_fatal (char const *msg)
 }
 
 static void
-ck_editor_status (int errnum, int status)
+check_child_status (int werrno, int wstatus, int max_ok_status)
 {
-  if (errnum | status)
-    {
-      error (0, errnum,
-            _(! errnum && WIFEXITED (status) && WEXITSTATUS (status) == 127
+  int status = (! werrno && WIFEXITED (wstatus)
+               ? WEXITSTATUS (wstatus)
+               : INT_MAX);
+
+  if (max_ok_status < status)
+    {
+      error (0, werrno,
+            _(status == 126
+              ? "subsidiary program `%s' could not be invoked"
+              : status == 127
               ? "subsidiary program `%s' not found"
-              : "subsidiary program `%s' failed"),
-            editor_program);
+              : status == INT_MAX
+              ? "subsidiary program `%s' failed"
+              : "subsidiary program `%s' failed (exit status %d)"),
+            editor_program, status);
       exiterr ();
     }
 }
@@ -658,7 +666,7 @@ main (int argc, char *argv[])
              }
 
            execvp (diffargv[0], (char **) diffargv);
-           _exit (127);
+           _exit (errno == ENOENT ? 127 : 126);
          }
 
 # if HAVE_WORKING_VFORK
@@ -718,7 +726,7 @@ main (int argc, char *argv[])
        if (! interact_ok)
          exiterr ();
 
-       ck_editor_status (werrno, wstatus);
+       check_child_status (werrno, wstatus, EXIT_FAILURE);
        untrapsig (0);
        checksigs ();
        exit (WEXITSTATUS (wstatus));
@@ -1064,7 +1072,7 @@ edit (struct line_filter *left, char con
                    argv[i] = 0;
 
                    execvp (editor_program, (char **) argv);
-                   _exit (127);
+                   _exit (errno == ENOENT ? 127 : 126);
                  }
 
                if (pid < 0)
@@ -1079,7 +1087,7 @@ edit (struct line_filter *left, char con
              }
 
              ignore_SIGINT = 0;
-             ck_editor_status (werrno, wstatus);
+             check_child_status (werrno, wstatus, EXIT_SUCCESS);
            }
 
            {