use execute module in m4

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Bruno, a while ago, you suggested that I replace m4's syscmd
implementation from system(3) over to the execute module.  I responded
that m4 needs to know the signal that killed the child process, so you
added a parameter to make that possible (2008-06-10).  I'm finally getting
around to changing m4 to use the module now, and encountered a regression,
where the execute module outputs an unwanted warning message when a signal
is encountered in the child; m4 would rather handle the warning itself to
give it consistent formatting.  OK to apply the gnulib patch?

For reference, here's what I'll probably be applying to m4 if the gnulib
patch is approved.  The m4 patch needs a followup to use
confstr(_CS_PATH,...) (where supported) and/or a configure.ac change to
store the user's preferred shell, rather than blindly calling "/bin/sh"
(on mingw, for example, the user will want "sh" or even "cmd", and not
"/bin/sh", as the preferred shell).  Also, I will write a followup to
convert esyscmd's use of popen into gnulib's pipe module.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkmqt6MACgkQ84KuGfSFAYDoLgCeKzQieE52/2p0l/b5cq6clUnN
lAUAoKde4qhuQm27hb4bafOOL2oBYQrz
=PLOe
-----END PGP SIGNATURE-----

>From d0b919b3a419118a177b2e6b2aa4c1b4f57dc3ad Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Sun, 1 Mar 2009 08:15:27 -0700
Subject: [PATCH] wait-process: don't warn about signal if termsigp was provided

* lib/wait-process.c (wait_subprocess): Silence warning about
signal if client can learn the signal value.
* tests/test-sched.c: Avoid compiler warnings.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog          |    7 +++++++
 lib/wait-process.c |    9 ++++-----
 tests/test-sched.c |    6 +++---
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 042250b..26392ef 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2009-03-01  Eric Blake  <address@hidden>
+
+       wait-process: don't warn about signal if termsigp was provided
+       * lib/wait-process.c (wait_subprocess): Silence warning about
+       signal if client can learn the signal value.
+       * tests/test-sched.c: Avoid compiler warnings.
+
 2009-03-01  Bruno Haible  <address@hidden>

        More support for FreeMiNT.
diff --git a/lib/wait-process.c b/lib/wait-process.c
index 7db25e9..6edd2d7 100644
--- a/lib/wait-process.c
+++ b/lib/wait-process.c
@@ -1,5 +1,5 @@
 /* Waiting for a subprocess to finish.
-   Copyright (C) 2001-2003, 2005-2008 Free Software Foundation, Inc.
+   Copyright (C) 2001-2003, 2005-2009 Free Software Foundation, Inc.
    Written by Bruno Haible <address@hidden>, 2001.

    This program is free software: you can redistribute it and/or modify
@@ -269,7 +269,7 @@ wait_subprocess (pid_t child, const char *progname,
       if (info.si_status == SIGPIPE && ignore_sigpipe)
        return 0;
 # endif
-      if (exit_on_error || !null_stderr)
+      if (exit_on_error || (!null_stderr && !termsigp))
        error (exit_on_error ? EXIT_FAILURE : 0, 0,
               _("%s subprocess got fatal signal %d"),
               progname, info.si_status);
@@ -288,11 +288,10 @@ wait_subprocess (pid_t child, const char *progname,
     }
 #else
   /* waitpid() is just as portable as wait() nowadays.  */
-  int status;
+  int status = 0;

   if (termsigp != NULL)
     *termsigp = 0;
-  *(int *) &status = 0;
   for (;;)
     {
       int result = waitpid (child, &status, 0);
@@ -341,7 +340,7 @@ wait_subprocess (pid_t child, const char *progname,
       if (WTERMSIG (status) == SIGPIPE && ignore_sigpipe)
        return 0;
 # endif
-      if (exit_on_error || !null_stderr)
+      if (exit_on_error || (!null_stderr && !termsigp))
        error (exit_on_error ? EXIT_FAILURE : 0, 0,
               _("%s subprocess got fatal signal %d"),
               progname, (int) WTERMSIG (status));
diff --git a/tests/test-sched.c b/tests/test-sched.c
index 8925d3a..4d49504 100644
--- a/tests/test-sched.c
+++ b/tests/test-sched.c
@@ -1,5 +1,5 @@
 /* Test of <sched.h> substitute.
-   Copyright (C) 2008 Free Software Foundation, Inc.
+   Copyright (C) 2008, 2009 Free Software Foundation, Inc.

    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -23,7 +23,7 @@
 /* Check that 'struct sched_param' is defined.  */
 static struct sched_param a;

-/* Check that the SCHED_* macris are defined and compile-time constants.  */
+/* Check that the SCHED_* macros are defined and compile-time constants.  */
 static int b[] = { SCHED_FIFO, SCHED_RR, SCHED_OTHER };

 static int f1;
@@ -32,7 +32,7 @@ int
 main ()
 {
   /* Check fields of 'struct sched_param'.  */
-  f1 = a.sched_priority;
+  f1 = a.sched_priority + b[0];

   return 0;
 }
-- 
1.6.1.2

>From ff4ad0e26fb2330b115d5e38027342131a6f989e Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Sun, 1 Mar 2009 09:02:06 -0700
Subject: [PATCH] Use gnulib execute module instead of system(3).

* m4/gnulib-cache.m4: Import execute module.
* src/builtin.c (m4_sysval): Move computation...
(m4_esyscmd): ...into caller.
(m4_syscmd): Rewrite with execute module.
Resolves a failure on AIX, reported by Gary V. Vaughan.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog          |    9 +++++++++
 m4/gnulib-cache.m4 |    3 ++-
 src/builtin.c      |   44 ++++++++++++++++++++++++--------------------
 3 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 696493c..8179daa 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2009-03-01  Eric Blake  <address@hidden>
+
+       Use gnulib execute module instead of system(3).
+       * m4/gnulib-cache.m4: Import execute module.
+       * src/builtin.c (m4_sysval): Move computation...
+       (m4_esyscmd): ...into caller.
+       (m4_syscmd): Rewrite with execute module.
+       Resolves a failure on AIX, reported by Gary V. Vaughan.
+
 2009-02-26  Eric Blake  <address@hidden>

        Make bootstrap easier on Solaris.
diff --git a/m4/gnulib-cache.m4 b/m4/gnulib-cache.m4
index 4ed53d0..6e99c32 100644
--- a/m4/gnulib-cache.m4
+++ b/m4/gnulib-cache.m4
@@ -15,7 +15,7 @@


 # Specification in the form of a command-line invocation:
-#   gnulib-tool --import --dir=. --local-dir=local --lib=libm4 
--source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests 
--aux-dir=build-aux --with-tests --avoid=lock-tests --avoid=tls-tests 
--no-libtool --macro-prefix=M4 announce-gen assert autobuild avltree-oset 
binary-io c-stack clean-temp cloexec close-stream closein config-h dirname 
error fdl-1.3 fflush filenamecat fopen fopen-safer fseeko gendocs getopt 
git-version-gen gnumakefile gnupload gpl-3.0 intprops memchr2 mkstemp obstack 
progname regex sigaction stdbool stdint stdlib-safer strsignal strstr strtod 
strtol unlocked-io verror version-etc version-etc-fsf xalloc xprintf 
xvasprintf-posix
+#   gnulib-tool --import --dir=. --local-dir=local --lib=libm4 
--source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests 
--aux-dir=build-aux --with-tests --avoid=lock-tests --avoid=tls-tests 
--no-libtool --macro-prefix=M4 announce-gen assert autobuild avltree-oset 
binary-io c-stack clean-temp cloexec close-stream closein config-h dirname 
error execute fdl-1.3 fflush filenamecat fopen fopen-safer fseeko gendocs 
getopt git-version-gen gnumakefile gnupload gpl-3.0 intprops memchr2 mkstemp 
obstack progname regex sigaction stdbool stdint stdlib-safer strsignal strstr 
strtod strtol unlocked-io verror version-etc version-etc-fsf xalloc xprintf 
xvasprintf-posix

 # Specification in the form of a few gnulib-tool.m4 macro invocations:
 gl_LOCAL_DIR([local])
@@ -33,6 +33,7 @@ gl_MODULES([
   config-h
   dirname
   error
+  execute
   fdl-1.3
   fflush
   filenamecat
diff --git a/src/builtin.c b/src/builtin.c
index 6378fb7..4645906 100644
--- a/src/builtin.c
+++ b/src/builtin.c
@@ -24,8 +24,7 @@

 #include "m4.h"

-extern FILE *popen ();
-
+#include "execute.h"
 #include "memchr2.h"
 #include "regex.h"

@@ -975,7 +974,11 @@ static int sysval;
 static void
 m4_syscmd (struct obstack *obs, int argc, token_data **argv)
 {
-  if (bad_argc (argv[0], argc, 2, 2))
+  const char *cmd = ARG (1);
+  int status;
+  int sig_status;
+  const char *prog_args[4] = { "sh", "-c" };
+  if (bad_argc (argv[0], argc, 2, 2) || !*cmd)
     {
       /* The empty command is successful.  */
       sysval = 0;
@@ -983,17 +986,18 @@ m4_syscmd (struct obstack *obs, int argc, token_data 
**argv)
     }

   debug_flush_files ();
-  sysval = system (ARG (1));
-#if FUNC_SYSTEM_BROKEN
-  /* OS/2 has a buggy system() that returns exit status in the lowest eight
-     bits, although pclose() and WEXITSTATUS are defined to return exit
-     status in the next eight bits.  This approach can't detect signals, but
-     at least syscmd(`ls') still works when stdout is a terminal.  An
-     alternate approach is popen/insert_file/pclose, but that makes stdout
-     a pipe, which can change how some child processes behave.  */
-  if (sysval != -1)
-    sysval <<= 8;
-#endif /* FUNC_SYSTEM_BROKEN */
+  prog_args[2] = cmd;
+  errno = 0;
+  status = execute (ARG (0), "/bin/sh"/*FIXME*/, (char **) prog_args, false,
+                   false, false, false, true, false, &sig_status);
+  if (status == 127 && sig_status)
+    {
+      if (errno)
+       M4ERROR ((warning_status, errno, "cannot run command `%s'", cmd));
+      sysval = sig_status << 8;
+    }
+  else
+    sysval = status;
 }

 static void
@@ -1013,9 +1017,8 @@ m4_esyscmd (struct obstack *obs, int argc, token_data 
**argv)
   pin = popen (ARG (1), "r");
   if (pin == NULL)
     {
-      M4ERROR ((warning_status, errno,
-               "cannot open pipe to command `%s'", ARG (1)));
-      sysval = -1;
+      M4ERROR ((warning_status, errno, "cannot run command `%s'", ARG (1)));
+      sysval = 127;
     }
   else
     {
@@ -1039,15 +1042,16 @@ m4_esyscmd (struct obstack *obs, int argc, token_data 
**argv)
       if (ferror (pin))
        M4ERROR ((EXIT_FAILURE, errno, "cannot read pipe"));
       sysval = pclose (pin);
+      sysval = (sysval == -1 ? 127
+               : (M4SYSVAL_EXITBITS (sysval)
+                  | M4SYSVAL_TERMSIGBITS (sysval)));
     }
 }

 static void
 m4_sysval (struct obstack *obs, int argc, token_data **argv)
 {
-  shipout_int (obs, (sysval == -1 ? 127
-                    : (M4SYSVAL_EXITBITS (sysval)
-                       | M4SYSVAL_TERMSIGBITS (sysval))));
+  shipout_int (obs, sysval);
 }
 
 /*-------------------------------------------------------------------------.
-- 
1.6.1.2