[bug-diffutils] new warnings due to use of vfork

[Jim Meyering]

Hi Paul,

I built diffutils today using gcc-4.6.0 on rawhide, and it failed
when configured with --enable-gcc-warnings:

sdiff.c: In function 'main':
sdiff.c:572:19: error: variable 'lname' might be clobbered by 'longjmp' or 
'vfork' [-Werror=clobbered]
sdiff.c:572:27: error: variable 'rname' might be clobbered by 'longjmp' or 
'vfork' [-Werror=clobbered]
cc1: all warnings being treated as errors

Also, considering that vfork is no longer even specified by POSIX.1-2008,
do you know of any reason to continue using it?

Here's a proposed patch:

>From f4b55fdffc3bb7019b17202b22d7e1c5eb37025e Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Mon, 24 Jan 2011 12:27:32 +0100
Subject: [PATCH] maint: remove all uses of vfork: use fork instead

Our use of vfork now provokes warnings from gcc-4.6.0.
Also, vfork is no longer even specified by POSIX.1-2008.
* src/diff3.c (read_diff): Change each use of vfork to "fork".
Remove #ifdef'd code.
* src/util.c (begin_output, finish_output): Likewise.
* src/sdiff.c (cleanup, main, edit): Likewise.
(handler_index_of_SIGPIPE): Remove now-unused definition.
* src/system.h: Don't include <vfork.h>.
(vfork): Remove definition.
* ms/config.site: Remove reference to vfork cache variable.
---
 ms/config.site |    1 -
 src/diff3.c    |    6 +++---
 src/sdiff.c    |   43 +++++++------------------------------------
 src/system.h   |    9 ---------
 src/util.c     |    8 ++++----
 5 files changed, 14 insertions(+), 53 deletions(-)

diff --git a/ms/config.site b/ms/config.site
index 4fa7712..80a475a 100644
--- a/ms/config.site
+++ b/ms/config.site
@@ -76,4 +76,3 @@ ac_cv_prog_LN_S='cp -pf'
 # We have `fork', but it always fails.  Don't trust Autoconf to be
 # smart enough to detect that...
 ac_cv_func_fork=no
-ac_cv_func_vfork=no
diff --git a/src/diff3.c b/src/diff3.c
index 44bc17f..b4e1e3a 100644
--- a/src/diff3.c
+++ b/src/diff3.c
@@ -1144,7 +1144,7 @@ read_diff (char const *filea,
   int werrno = 0;
   struct stat pipestat;

-#if HAVE_WORKING_FORK || HAVE_WORKING_VFORK
+#if HAVE_WORKING_FORK

   char const *argv[9];
   char const **ap;
@@ -1166,7 +1166,7 @@ read_diff (char const *filea,
   if (pipe (fds) != 0)
     perror_with_exit ("pipe");

-  pid = vfork ();
+  pid = fork ();
   if (pid == 0)
     {
       /* Child */
@@ -1255,7 +1255,7 @@ read_diff (char const *filea,

   *output_placement = diff_result;

-#if ! (HAVE_WORKING_FORK || HAVE_WORKING_VFORK)
+#if ! HAVE_WORKING_FORK

   wstatus = pclose (fpipe);
   if (wstatus == -1)
diff --git a/src/sdiff.c b/src/sdiff.c
index eb835c3..d2b1f3c 100644
--- a/src/sdiff.c
+++ b/src/sdiff.c
@@ -50,7 +50,7 @@ static char const **diffargv;
 static char * volatile tmpname;
 static FILE *tmp;

-#if HAVE_WORKING_FORK || HAVE_WORKING_VFORK
+#if HAVE_WORKING_FORK
 static pid_t volatile diffpid;
 #endif

@@ -85,7 +85,6 @@ static int const sigs[] = {
 #endif
 #ifdef SIGPIPE
        SIGPIPE,
-# define handler_index_of_SIGPIPE (NUM_SIGS - 2)
 #endif
        SIGINT
 #define handler_index_of_SIGINT (NUM_SIGS - 1)
@@ -217,7 +216,7 @@ usage (void)
 static void
 cleanup (int signo __attribute__((unused)))
 {
-#if HAVE_WORKING_FORK || HAVE_WORKING_VFORK
+#if HAVE_WORKING_FORK
   if (0 < diffpid)
     kill (diffpid, SIGPIPE);
 #endif
@@ -595,7 +594,7 @@ main (int argc, char *argv[])

       trapsigs ();

-#if ! (HAVE_WORKING_FORK || HAVE_WORKING_VFORK)
+#if ! HAVE_WORKING_FORK
       {
        size_t cmdsize = 1;
        char *p, *command;
@@ -619,22 +618,11 @@ main (int argc, char *argv[])
 #else
       {
        int diff_fds[2];
-# if HAVE_WORKING_VFORK
-       sigset_t procmask;
-       sigset_t blocked;
-# endif

        if (pipe (diff_fds) != 0)
          perror_fatal ("pipe");

-# if HAVE_WORKING_VFORK
-       /* Block SIGINT and SIGPIPE.  */
-       sigemptyset (&blocked);
-       sigaddset (&blocked, SIGINT);
-       sigaddset (&blocked, SIGPIPE);
-       sigprocmask (SIG_BLOCK, &blocked, &procmask);
-# endif
-       diffpid = vfork ();
+       diffpid = fork ();
        if (diffpid < 0)
          perror_fatal ("fork");
        if (! diffpid)
@@ -646,10 +634,6 @@ main (int argc, char *argv[])
            if (initial_handler (handler_index_of_SIGINT) != SIG_IGN)
              signal_handler (SIGINT, SIG_IGN);
            signal_handler (SIGPIPE, SIG_DFL);
-# if HAVE_WORKING_VFORK
-           /* Stop blocking SIGINT and SIGPIPE in the child.  */
-           sigprocmask (SIG_SETMASK, &procmask, 0);
-# endif
            close (diff_fds[0]);
            if (diff_fds[1] != STDOUT_FILENO)
              {
@@ -661,19 +645,6 @@ main (int argc, char *argv[])
            _exit (errno == ENOENT ? 127 : 126);
          }

-# if HAVE_WORKING_VFORK
-       /* Restore the parent's SIGINT and SIGPIPE behavior.  */
-       if (initial_handler (handler_index_of_SIGINT) != SIG_IGN)
-         signal_handler (SIGINT, catchsig);
-       if (initial_handler (handler_index_of_SIGPIPE) != SIG_IGN)
-         signal_handler (SIGPIPE, catchsig);
-       else
-         signal_handler (SIGPIPE, SIG_IGN);
-
-       /* Stop blocking SIGINT and SIGPIPE in the parent.  */
-       sigprocmask (SIG_SETMASK, &procmask, 0);
-# endif
-
        close (diff_fds[1]);
        diffout = fdopen (diff_fds[0], "r");
        if (! diffout)
@@ -695,7 +666,7 @@ main (int argc, char *argv[])
        int wstatus;
        int werrno = 0;

-#if ! (HAVE_WORKING_FORK || HAVE_WORKING_VFORK)
+#if ! HAVE_WORKING_FORK
        wstatus = pclose (diffout);
        if (wstatus == -1)
          werrno = errno;
@@ -1042,7 +1013,7 @@ edit (struct line_filter *left, char const *lname, lin 
lline, lin llen,
              checksigs ();

              {
-#if ! (HAVE_WORKING_FORK || HAVE_WORKING_VFORK)
+#if ! HAVE_WORKING_FORK
                char *command =
                  xmalloc (shell_quote_length (editor_program)
                           + 1 + strlen (tmpname) + 1);
@@ -1055,7 +1026,7 @@ edit (struct line_filter *left, char const *lname, lin 
lline, lin llen,
 #else
                pid_t pid;

-               pid = vfork ();
+               pid = fork ();
                if (pid == 0)
                  {
                    char const *argv[3];
diff --git a/src/system.h b/src/system.h
index 70e4a7e..15656bc 100644
--- a/src/system.h
+++ b/src/system.h
@@ -115,15 +115,6 @@ int strcasecmp (char const *, char const *);
 #define MAX(a, b) ((a) >= (b) ? (a) : (b))

 #include <stdbool.h>
-
-#if HAVE_VFORK_H
-# include <vfork.h>
-#endif
-
-#if ! HAVE_WORKING_VFORK
-# define vfork fork
-#endif
-
 #include <intprops.h>
 #include "propername.h"

diff --git a/src/util.c b/src/util.c
index 867419c..bba51a5 100644
--- a/src/util.c
+++ b/src/util.c
@@ -162,7 +162,7 @@ setup_output (char const *name0, char const *name1, bool 
recursive)
   outfile = 0;
 }

-#if HAVE_WORKING_FORK || HAVE_WORKING_VFORK
+#if HAVE_WORKING_FORK
 static pid_t pr_pid;
 #endif

@@ -192,13 +192,13 @@ begin_output (void)

       /* Make OUTFILE a pipe to a subsidiary `pr'.  */
       {
-#if HAVE_WORKING_FORK || HAVE_WORKING_VFORK
+#if HAVE_WORKING_FORK
        int pipes[2];

        if (pipe (pipes) != 0)
          pfatal_with_name ("pipe");

-       pr_pid = vfork ();
+       pr_pid = fork ();
        if (pr_pid < 0)
          pfatal_with_name ("fork");

@@ -282,7 +282,7 @@ finish_output (void)
       int werrno = 0;
       if (ferror (outfile))
        fatal ("write failed");
-#if ! (HAVE_WORKING_FORK || HAVE_WORKING_VFORK)
+#if ! HAVE_WORKING_FORK
       wstatus = pclose (outfile);
       if (wstatus == -1)
        werrno = errno;
--
1.7.3.5.38.gb312b