m4-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: use execute module in m4


From: Eric Blake
Subject: Re: use execute module in m4
Date: Tue, 03 Mar 2009 06:39:02 -0700
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.19) Gecko/20081209 Thunderbird/2.0.0.19 Mnenhy/0.7.6.666

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

According to Eric Blake on 3/1/2009 9:28 AM:
> 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.

Here's the followup patches which switch to the pipe module for esyscmd,
nuke code rendered dead now that gnulib takes care of separating signal
from normal exit status, then adds ./configure --with-syscmd-shell to
allow overriding the default choice of /bin/sh (or, on Solaris,
/usr/xpg4/bin/sh).  I've tested this on several platforms, but won't push
to savannah until I've ported it to branch-1.6 and master.  To test it
now, you can:

$ git pull git://repo.or.cz/m4/ericb.git branch-1.4

Gary, I'd be particularly interested in seeing if this cleans up the
testsuite failures you saw on AIX related to sysval (since I already made
that assumption when writing the ChangeLog).

- --
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

iEYEARECAAYFAkmtMvYACgkQ84KuGfSFAYDZVwCfUSOrj0f4SS9ueGS5Amt1cN3T
WgYAnjEpzE/N5orf6zAUl5i97yuD9srI
=s+1V
-----END PGP SIGNATURE-----
>From 60e6aa65c4187b1ced9beb8bed35de371d30969a Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Mon, 2 Mar 2009 06:54:28 -0700
Subject: [PATCH 1/3] Use gnulib pipe module instead of popen(3).

* m4/gnulib-cache.m4: Import pipe and wait-process modules.
* src/builtin.c (m4_esyscmd): Rewrite with pipe module.
Resolves a failure on AIX, reported by Gary V. Vaughan.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog          |    5 +++
 m4/gnulib-cache.m4 |    4 ++-
 src/builtin.c      |   73 +++++++++++++++++++++++++++++++++++-----------------
 3 files changed, 57 insertions(+), 25 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 6ce7f5b..7fbeacb 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2009-03-02  Eric Blake  <address@hidden>

+       Use gnulib pipe module instead of popen(3).
+       * m4/gnulib-cache.m4: Import pipe and wait-process modules.
+       * src/builtin.c (m4_esyscmd): Rewrite with pipe module.
+       Resolves a failure on AIX, reported by Gary V. Vaughan.
+
        Use gnulib execute module instead of system(3).
        * m4/gnulib-cache.m4: Import execute module.
        * src/builtin.c (m4_sysval): Move computation...
diff --git a/m4/gnulib-cache.m4 b/m4/gnulib-cache.m4
index 6e99c32..3c59620 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 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
+#   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 pipe progname regex sigaction stdbool stdint stdlib-safer strsignal 
strstr strtod strtol unlocked-io verror version-etc version-etc-fsf 
wait-process xalloc xprintf xvasprintf-posix

 # Specification in the form of a few gnulib-tool.m4 macro invocations:
 gl_LOCAL_DIR([local])
@@ -50,6 +50,7 @@ gl_MODULES([
   memchr2
   mkstemp
   obstack
+  pipe
   progname
   regex
   sigaction
@@ -64,6 +65,7 @@ gl_MODULES([
   verror
   version-etc
   version-etc-fsf
+  wait-process
   xalloc
   xprintf
   xvasprintf-posix
diff --git a/src/builtin.c b/src/builtin.c
index 292c2d6..363cbcb 100644
--- a/src/builtin.c
+++ b/src/builtin.c
@@ -26,7 +26,9 @@

 #include "execute.h"
 #include "memchr2.h"
+#include "pipe.h"
 #include "regex.h"
+#include "wait-process.h"

 #if HAVE_SYS_WAIT_H
 # include <sys/wait.h>
@@ -1006,9 +1008,15 @@ m4_syscmd (struct obstack *obs, int argc, token_data 
**argv)
 static void
 m4_esyscmd (struct obstack *obs, int argc, token_data **argv)
 {
+  const char *cmd = ARG (1);
+  const char *prog_args[4] = { "sh", "-c" };
+  pid_t child;
+  int fd;
   FILE *pin;
+  int status;
+  int sig_status;

-  if (bad_argc (argv[0], argc, 2, 2))
+  if (bad_argc (argv[0], argc, 2, 2) || !*cmd)
     {
       /* The empty command is successful.  */
       sysval = 0;
@@ -1016,38 +1024,55 @@ m4_esyscmd (struct obstack *obs, int argc, token_data 
**argv)
     }

   debug_flush_files ();
+  prog_args[2] = cmd;
   errno = 0;
-  pin = popen (ARG (1), "r");
+  child = create_pipe_in (ARG (0), "/bin/sh"/*FIXME*/, (char **) prog_args,
+                         NULL, false, true, false, &fd);
+  if (child == -1)
+    {
+      M4ERROR ((warning_status, errno, "cannot run command `%s'", cmd));
+      sysval = 127;
+      return;
+    }
+  pin = fdopen (fd, "r");
   if (pin == NULL)
     {
-      M4ERROR ((warning_status, errno, "cannot run command `%s'", ARG (1)));
+      M4ERROR ((warning_status, errno, "cannot run command `%s'", cmd));
       sysval = 127;
+      close (fd);
+      return;
     }
-  else
+  while (1)
     {
-      while (1)
+      size_t avail = obstack_room (obs);
+      size_t len;
+      if (!avail)
        {
-         size_t avail = obstack_room (obs);
-         size_t len;
-         if (!avail)
-           {
-             int ch = getc (pin);
-             if (ch == EOF)
-               break;
-             obstack_1grow (obs, ch);
-             continue;
-           }
-         len = fread (obstack_next_free (obs), 1, avail, pin);
-         if (len <= 0)
+         int ch = getc (pin);
+         if (ch == EOF)
            break;
-         obstack_blank_fast (obs, len);
+         obstack_1grow (obs, ch);
+         continue;
        }
-      if (ferror (pin))
-       M4ERROR ((EXIT_FAILURE, errno, "cannot read pipe"));
-      sysval = pclose (pin);
-      sysval = (sysval == -1 ? 127
-               : (M4SYSVAL_EXITBITS (sysval)
-                  | M4SYSVAL_TERMSIGBITS (sysval)));
+      len = fread (obstack_next_free (obs), 1, avail, pin);
+      if (len <= 0)
+       break;
+      obstack_blank_fast (obs, len);
+    }
+  if (ferror (pin) || fclose (pin))
+    M4ERROR ((EXIT_FAILURE, errno, "cannot read pipe"));
+  status = wait_subprocess (child, ARG (0), false, false, true, false,
+                           &sig_status);
+  if (sig_status)
+    {
+      assert (status == 127);
+      sysval = sig_status << 8;
+    }
+  else
+    {
+      if (status == 127 && errno)
+       M4ERROR ((warning_status, errno, "cannot run command `%s'", cmd));
+      sysval = status;
     }
 }

-- 
1.6.1.2


>From 82f333b05ff4d00947b699b9a326375db6b54eb7 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Mon, 2 Mar 2009 07:22:17 -0700
Subject: [PATCH 2/3] Remove cruft now that gnulib modules do the work.

* configure.ac (M4_cv_func_system_consistent): Delete.
* src/builtin.c (M4SYSVAL_EXITBITS, M4SYSVAL_TERMSIGBITS):
Delete.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog     |    5 +++++
 configure.ac  |   36 ++----------------------------------
 src/builtin.c |   33 ---------------------------------
 3 files changed, 7 insertions(+), 67 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 7fbeacb..6925ccb 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2009-03-02  Eric Blake  <address@hidden>

+       Remove cruft now that gnulib modules do the work.
+       * configure.ac (M4_cv_func_system_consistent): Delete.
+       * src/builtin.c (M4SYSVAL_EXITBITS, M4SYSVAL_TERMSIGBITS):
+       Delete.
+
        Use gnulib pipe module instead of popen(3).
        * m4/gnulib-cache.m4: Import pipe and wait-process modules.
        * src/builtin.c (m4_esyscmd): Rewrite with pipe module.
diff --git a/configure.ac b/configure.ac
index f728b52..069438f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,6 +1,6 @@
 # Configure template for GNU M4.                       -*-Autoconf-*-
-# Copyright (C) 1991, 1993, 1994, 2004, 2005, 2006, 2007, 2008 Free
-# Software Foundation, Inc.
+# Copyright (C) 1991, 1993, 1994, 2004, 2005, 2006, 2007, 2008, 2009
+# Free Software Foundation, Inc.
 #
 # This file is part of GNU M4.
 #
@@ -49,38 +49,6 @@ fi

 M4_INIT

-AC_CACHE_CHECK([if system() agrees with pclose()],
-  [M4_cv_func_system_consistent],
-  [AC_RUN_IFELSE([AC_LANG_PROGRAM([
-#include <stdio.h>
-#include <stdlib.h>
-#include <errno.h>
-], [int i1, i2;
-  FILE *f;
-  i1 = system ("exit 2");
-  if (i1 == -1)
-    return 1;
-  f = popen ("exit 2", "r");
-  if (!f)
-    return 1;
-  i2 = pclose (f);
-  return i1 != i2;])],
-  [M4_cv_func_system_consistent=yes], [M4_cv_func_system_consistent=no],
-  [AC_COMPILE_IFELSE([
-/* EMX on OS/2 defines WEXITSTATUS to be (x>>8)&0xff, and uses that for
-   pclose(), but for system() it uses x&0xff instead.  Otherwise, we assume
-   your system is sane and that pclose() and system() are consistent in their
-   values.  If this heuristic is wrong for your platform, report it as a bug
-   to address@hidden  */
-#ifdef __EMX__
-choke me
-#endif
-], [M4_cv_func_system_consistent=yes], [M4_cv_func_system_consistent=no])])])
-if test "$M4_cv_func_system_consistent" = no ; then
-  AC_DEFINE([FUNC_SYSTEM_BROKEN], [1],
-    [Define to 1 if the return value of system() disagrees with pclose().])
-fi
-
 AC_CACHE_CHECK([whether an open file can be renamed],
   [M4_cv_func_rename_open_file_works],
   [AC_RUN_IFELSE([AC_LANG_PROGRAM([AC_INCLUDES_DEFAULT],
diff --git a/src/builtin.c b/src/builtin.c
index 363cbcb..219883d 100644
--- a/src/builtin.c
+++ b/src/builtin.c
@@ -30,10 +30,6 @@
 #include "regex.h"
 #include "wait-process.h"

-#if HAVE_SYS_WAIT_H
-# include <sys/wait.h>
-#endif
-
 #define ARG(i) (argc > (i) ? TOKEN_DATA_TEXT (argv[i]) : "")

 /* Initialization of builtin and predefined macros.  The table
@@ -941,35 +937,6 @@ builtin `%s' requested by frozen file is not supported", 
ARG (i)));
 | and "sysval".  "esyscmd" is GNU specific.                              |
 `------------------------------------------------------------------------*/

-/* Helper macros for readability.  */
-#if UNIX || defined WEXITSTATUS
-# define M4SYSVAL_EXITBITS(status)                       \
-   (WIFEXITED (status) ? WEXITSTATUS (status) : 0)
-# define M4SYSVAL_TERMSIGBITS(status)                    \
-   (WIFSIGNALED (status) ? WTERMSIG (status) << 8 : 0)
-
-#else /* ! UNIX && ! defined WEXITSTATUS */
-/* Platforms such as mingw do not support the notion of reporting
-   which signal terminated a process.  Furthermore if WEXITSTATUS was
-   not provided, then the exit value is in the low eight bits.  */
-# define M4SYSVAL_EXITBITS(status) status
-# define M4SYSVAL_TERMSIGBITS(status) 0
-#endif /* ! UNIX && ! defined WEXITSTATUS */
-
-/* Fallback definitions if <stdlib.h> or <sys/wait.h> are inadequate.  */
-#ifndef WEXITSTATUS
-# define WEXITSTATUS(status) (((status) >> 8) & 0xff)
-#endif
-#ifndef WTERMSIG
-# define WTERMSIG(status) ((status) & 0x7f)
-#endif
-#ifndef WIFSIGNALED
-# define WIFSIGNALED(status) (WTERMSIG (status) != 0)
-#endif
-#ifndef WIFEXITED
-# define WIFEXITED(status) (WTERMSIG (status) == 0)
-#endif
-
 /* Exit code from last "syscmd" command.  */
 static int sysval;

-- 
1.6.1.2


>From ee1c6099d67bd7e359790943322fb0239d1b667b Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Mon, 2 Mar 2009 15:08:31 -0700
Subject: [PATCH 3/3] Allow configuration choice in syscmd shell.

* configure.ac (SYSCMD_SHELL): New test.
* src/builtin.c (m4_syscmd, m4_esyscmd): Use it to avoid
hard-coding the shell location.
* doc/m4.texinfo (Syscmd, Esyscmd): Document this.
* README: Document new configure option.
* NEWS: Likewise.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog      |    8 ++++++++
 NEWS           |    3 +++
 README         |   10 ++++++++--
 configure.ac   |   29 +++++++++++++++++++++++++++++
 doc/m4.texinfo |   21 ++++++++++++++++++++-
 src/builtin.c  |    4 ++--
 6 files changed, 70 insertions(+), 5 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 6925ccb..1d52557 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2009-03-02  Eric Blake  <address@hidden>

+       Allow configuration choice in syscmd shell.
+       * configure.ac (SYSCMD_SHELL): New test.
+       * src/builtin.c (m4_syscmd, m4_esyscmd): Use it to avoid
+       hard-coding the shell location.
+       * doc/m4.texinfo (Syscmd, Esyscmd): Document this.
+       * README: Document new configure option.
+       * NEWS: Likewise.
+
        Remove cruft now that gnulib modules do the work.
        * configure.ac (M4_cv_func_system_consistent): Delete.
        * src/builtin.c (M4SYSVAL_EXITBITS, M4SYSVAL_TERMSIGBITS):
diff --git a/NEWS b/NEWS
index 22ea347..fa6832b 100644
--- a/NEWS
+++ b/NEWS
@@ -22,6 +22,9 @@ Software Foundation, Inc.
    `m4 --debugfile=trace file', not `m4 file --debugfile trace'.  This
    change does not affect the deprecated `-o'/`--error-output' option.

+** The `syscmd' and `esyscmd' builtins can be configured to use an
+   alternate shell, via the new `configure' option `--with-syscmd-shell'.
+
 ** A number of portability improvements inherited from gnulib.

 * Noteworthy changes in Version 1.4.12 (2008-10-10) [stable]
diff --git a/README b/README
index 49275d2..2197a2c 100644
--- a/README
+++ b/README
@@ -44,6 +44,12 @@ See file `INSTALL' for compilation and installation 
instructions.
 See file `NEWS' for a list of major changes in the current release.
 See file `THANKS' for a list of contributors.

+By default, the `syscmd' and `esyscmd' macros try to use the first
+instance of `sh' found by `command -p getconf PATH' at configure time,
+with a default of `/bin/sh'.  If that default is inappropriate, you
+can use `./configure --with-syscmd-shell=location' to specify the
+shell to use.
+
 By using `./configure --enable-changeword', you get an experimental
 feature which allows for changing the syntax of what is a "word" in
 `m4'.  This feature will not be present in m4 2.0, but will be
@@ -57,8 +63,8 @@ solution, from which the problem might be uneasy to infer.

 ========================================================================

-Copyright (C) 2000, 2005, 2006, 2007, 2008 Free Software Foundation,
-Inc.
+Copyright (C) 2000, 2005, 2006, 2007, 2008, 2009 Free Software
+Foundation, Inc.

 Permission is granted to copy, distribute and/or modify this document
 under the terms of the GNU Free Documentation License, Version 1.2 or
diff --git a/configure.ac b/configure.ac
index 069438f..c37cac9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -83,6 +83,35 @@ AC_ARG_ENABLE([changeword],
     AC_MSG_RESULT([no])
   fi], [AC_MSG_RESULT([no])])

+AC_MSG_CHECKING([[which shell to use for syscmd]])
+AC_ARG_WITH([syscmd-shell],
+  [AS_HELP_STRING([--with-syscmd-shell], [shell used by syscmd [/bin/sh]])],
+  [case $withval in
+    yes[)] with_syscmd_shell=no;;
+   esac], [with_syscmd_shell=no])
+if test "$with_syscmd_shell" = no ; then
+  with_syscmd_shell=/bin/sh
+  if test "$cross_compiling" != yes ; then
+dnl Too bad _AS_PATH_WALK is not public.
+    M4_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+    for M4_dir in `if (command -p getconf PATH) 2>/dev/null ; then
+       command -p getconf PATH
+      else
+       echo "/bin$PATH_SEPARATOR$PATH"
+      fi`
+    do
+      IFS=$M4_save_IFS
+      test -z "$M4_dir" && continue
+      AS_EXECUTABLE_P(["$M4_dir/sh"]) \
+       && { with_syscmd_shell=$M4_dir/sh; break; }
+    done
+    IFS=$M4_save_IFS
+  fi
+fi
+AC_MSG_RESULT([$with_syscmd_shell])
+AC_DEFINE_UNQUOTED([SYSCMD_SHELL], ["$with_syscmd_shell"],
+  [Shell used by syscmd and esyscmd, must accept -c argument.])
+
 M4_WITH_DMALLOC

 AC_CONFIG_COMMANDS([stamp-h], [[test -z "$CONFIG_HEADERS" || date > stamp-h]])
diff --git a/doc/m4.texinfo b/doc/m4.texinfo
index 6877ecc..a30d2cf 100644
--- a/doc/m4.texinfo
+++ b/doc/m4.texinfo
@@ -4552,7 +4552,8 @@ Changeword
 @quotation
 The macro @code{changeword} and all associated functionality is
 experimental.  It is only available if the @option{--enable-changeword}
-option was given to @code{configure}, at @acronym{GNU} @code{m4} installation
+option was given to @command{configure}, at @acronym{GNU} @code{m4}
+installation
 time.  The functionality will go away in the future, to be replaced by
 other new features that are more efficient at providing the same
 capabilities.  @emph{Do not rely on it}.  Please direct your comments
@@ -6477,6 +6478,15 @@ Syscmd
 The default standard input, output and error of @var{shell-command} are
 the same as those of @code{m4}.

+By default, the @var{shell-command} will be used as the argument to the
address@hidden option of the @command{/bin/sh} shell (or the version of
address@hidden specified by @samp{command -p getconf PATH}, if your system
+supports that).  If you prefer a different shell, the
address@hidden script can be given the option
address@hidden@var{location}} to set the location of an
+alternative shell at @acronym{GNU} @code{m4} installation; the
+alternative shell must still support @option{-c}.
+
 The macro @code{syscmd} is recognized only with parameters.
 @end deffn

@@ -6545,6 +6555,15 @@ Esyscmd
 is not a part of the expansion: it will appear along with the error
 output of @code{m4}.

+By default, the @var{shell-command} will be used as the argument to the
address@hidden option of the @command{/bin/sh} shell (or the version of
address@hidden specified by @samp{command -p getconf PATH}, if your system
+supports that).  If you prefer a different shell, the
address@hidden script can be given the option
address@hidden@var{location}} to set the location of an
+alternative shell at @acronym{GNU} @code{m4} installation; the
+alternative shell must still support @option{-c}.
+
 The macro @code{esyscmd} is recognized only with parameters.
 @end deffn

diff --git a/src/builtin.c b/src/builtin.c
index 219883d..a0a1fac 100644
--- a/src/builtin.c
+++ b/src/builtin.c
@@ -957,7 +957,7 @@ m4_syscmd (struct obstack *obs, int argc, token_data **argv)
   debug_flush_files ();
   prog_args[2] = cmd;
   errno = 0;
-  status = execute (ARG (0), "/bin/sh"/*FIXME*/, (char **) prog_args, false,
+  status = execute (ARG (0), SYSCMD_SHELL, (char **) prog_args, false,
                    false, false, false, true, false, &sig_status);
   if (sig_status)
     {
@@ -993,7 +993,7 @@ m4_esyscmd (struct obstack *obs, int argc, token_data 
**argv)
   debug_flush_files ();
   prog_args[2] = cmd;
   errno = 0;
-  child = create_pipe_in (ARG (0), "/bin/sh"/*FIXME*/, (char **) prog_args,
+  child = create_pipe_in (ARG (0), SYSCMD_SHELL, (char **) prog_args,
                          NULL, false, true, false, &fd);
   if (child == -1)
     {
-- 
1.6.1.2


reply via email to

[Prev in Thread] Current Thread [Next in Thread]