autoconf-patches
[Top][All Lists]
Advanced

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

[PATCH] m4sh: avoid // issues in _AS_PATH_WALK


From: Eric Blake
Subject: [PATCH] m4sh: avoid // issues in _AS_PATH_WALK
Date: Fri, 13 Jul 2012 10:56:43 -0600

As reported by Paul Keir on the cygwin lists,
http://cygwin.com/ml/cygwin/2012-07/msg00263.html,
some people like to stick / in their $PATH, and if we then try
to probe $as_dir/progname for existence, we can end up causing
cygwin to have a several-second timeout per //name probe.  It
is better to avoid inserting the extra slash when $as_dir is the
root directory, and simpler to code by always having a trailing
slash present than it is to strip a trailing slash.  Thankfully,
_AS_PATH_WALK is an undocumented interface, and even if someone
was using it in spite of the warnings, their use of $as_dir/foo
will only lead to odd-looking /dir//foo probes.

There was also a minor bug where the if-not-found code of
_AS_PATH_WALK could be executed with $IFS still in the wrong state.

* lib/m4sugar/m4sh.m4 (_AS_PATH_WALK): Always end as_dir in /.
Avoid wrong IFS during if-not-found.  Minor optimization to avoid
regex.
(_AS_DETECT_BETTER_SHELL, _AS_SHELL_SANITIZE): Update clients.
* lib/autotest/general.m4 (_AT_FINISH): Likewise.
* lib/autoconf/programs.m4 (_AC_CHECK_PROG, _AC_PATH_PROG)
(_AC_PATH_PROGS_FEATURE_CHECK, _AC_PATH_PROG_FLAVOR_GNU): Likewise.
---

I will apply this in a couple days, or sooner if I get a favorable
review.  Of course, it won't help all the existing packages that
were built with older autoconf, but every little bit helps.

 lib/autoconf/programs.m4 |   38 +++++++++++++++++++-------------------
 lib/autotest/general.m4  |    4 ++--
 lib/m4sugar/m4sh.m4      |   22 +++++++++++++---------
 3 files changed, 34 insertions(+), 30 deletions(-)

diff --git a/lib/autoconf/programs.m4 b/lib/autoconf/programs.m4
index 2013a7a..a487db7 100644
--- a/lib/autoconf/programs.m4
+++ b/lib/autoconf/programs.m4
@@ -49,14 +49,14 @@ m4_ifvaln([$6],
 [  ac_prog_rejected=no])dnl
 _AS_PATH_WALK([$5],
 [for ac_exec_ext in '' $ac_executable_extensions; do
-  if AS_EXECUTABLE_P(["$as_dir/$ac_word$ac_exec_ext"]); then
+  if AS_EXECUTABLE_P(["$as_dir$ac_word$ac_exec_ext"]); then
 m4_ifvaln([$6],
-[    if test "$as_dir/$ac_word$ac_exec_ext" = "$6"; then
+[    if test "$as_dir$ac_word$ac_exec_ext" = "$6"; then
        ac_prog_rejected=yes
        continue
      fi])dnl
     ac_cv_prog_$1="$3"
-    _AS_ECHO_LOG([found $as_dir/$ac_word$ac_exec_ext])
+    _AS_ECHO_LOG([found $as_dir$ac_word$ac_exec_ext])
     break 2
   fi
 done])
@@ -70,7 +70,7 @@ m4_ifvaln([$6],
     # However, it has the same basename, so the bogon will be chosen
     # first if we set $1 to just the basename; use the full file name.
     shift
-    ac_cv_prog_$1="$as_dir/$ac_word${1+' 'address@hidden"
+    ac_cv_prog_$1="$as_dir$ac_word${1+' 'address@hidden"
 m4_if([$2], [$4],
 [  else
     # Default is a loser.
@@ -129,9 +129,9 @@ AC_CACHE_VAL([ac_cv_path_$1],
   *)
   _AS_PATH_WALK([$4],
 [for ac_exec_ext in '' $ac_executable_extensions; do
-  if AS_EXECUTABLE_P(["$as_dir/$ac_word$ac_exec_ext"]); then
-    ac_cv_path_$1="$as_dir/$ac_word$ac_exec_ext"
-    _AS_ECHO_LOG([found $as_dir/$ac_word$ac_exec_ext])
+  if AS_EXECUTABLE_P(["$as_dir$ac_word$ac_exec_ext"]); then
+    ac_cv_path_$1="$as_dir$ac_word$ac_exec_ext"
+    _AS_ECHO_LOG([found $as_dir$ac_word$ac_exec_ext])
     break 2
   fi
 done])
@@ -423,7 +423,7 @@ m4_define([_AC_PATH_PROGS_FEATURE_CHECK],
   _AS_PATH_WALK([$5],
   [for ac_prog in $2; do
     for ac_exec_ext in '' $ac_executable_extensions; do
-      ac_path_$1="$as_dir/$ac_prog$ac_exec_ext"
+      ac_path_$1="$as_dir$ac_prog$ac_exec_ext"
       AS_EXECUTABLE_P(["$ac_path_$1"]) || continue
 $3
       $ac_path_$1_found && break 3
@@ -542,9 +542,9 @@ AC_MSG_CHECKING([for a BSD-compatible install])
 if test -z "$INSTALL"; then
 AC_CACHE_VAL(ac_cv_path_install,
 [_AS_PATH_WALK([$PATH],
-[[# Account for people who put trailing slashes in PATH elements.
-case $as_dir/ in @%:@((
-  ./ | .// | /[cC]/* | \
+[[# Account for fact that we put trailing slashes in our PATH walk.
+case $as_dir in @%:@((
+  ./ | /[cC]/* | \
   /etc/* | /usr/sbin/* | /usr/etc/* | /sbin/* | /usr/afsws/bin/* | \
   ?:[\\/]os2[\\/]install[\\/]* | ?:[\\/]OS2[\\/]INSTALL[\\/]* | \
   /usr/ucb/* ) ;;
@@ -554,13 +554,13 @@ case $as_dir/ in @%:@((
     # by default.
     for ac_prog in ginstall scoinst install; do
       for ac_exec_ext in '' $ac_executable_extensions; do
-       if AS_EXECUTABLE_P(["$as_dir/$ac_prog$ac_exec_ext"]); then
+       if AS_EXECUTABLE_P(["$as_dir$ac_prog$ac_exec_ext"]); then
          if test $ac_prog = install &&
-           grep dspmsg "$as_dir/$ac_prog$ac_exec_ext" >/dev/null 2>&1; then
+           grep dspmsg "$as_dir$ac_prog$ac_exec_ext" >/dev/null 2>&1; then
            # AIX install.  It has an incompatible calling convention.
            :
          elif test $ac_prog = install &&
-           grep pwplus "$as_dir/$ac_prog$ac_exec_ext" >/dev/null 2>&1; then
+           grep pwplus "$as_dir$ac_prog$ac_exec_ext" >/dev/null 2>&1; then
            # program-specific install script used by HP pwplus--don't use.
            :
          else
@@ -568,12 +568,12 @@ case $as_dir/ in @%:@((
            echo one > conftest.one
            echo two > conftest.two
            mkdir conftest.dir
-           if "$as_dir/$ac_prog$ac_exec_ext" -c conftest.one conftest.two 
"`pwd`/conftest.dir" &&
+           if "$as_dir$ac_prog$ac_exec_ext" -c conftest.one conftest.two 
"`pwd`/conftest.dir" &&
              test -s conftest.one && test -s conftest.two &&
              test -s conftest.dir/conftest.one &&
              test -s conftest.dir/conftest.two
            then
-             ac_cv_path_install="$as_dir/$ac_prog$ac_exec_ext -c"
+             ac_cv_path_install="$as_dir$ac_prog$ac_exec_ext -c"
              break 3
            fi
          fi
@@ -668,12 +668,12 @@ if test -z "$MKDIR_P"; then
     [_AS_PATH_WALK([$PATH$PATH_SEPARATOR/opt/sfw/bin],
       [for ac_prog in mkdir gmkdir; do
         for ac_exec_ext in '' $ac_executable_extensions; do
-          AS_EXECUTABLE_P(["$as_dir/$ac_prog$ac_exec_ext"]) || continue
-          case `"$as_dir/$ac_prog$ac_exec_ext" --version 2>&1` in #(
+          AS_EXECUTABLE_P(["$as_dir$ac_prog$ac_exec_ext"]) || continue
+          case `"$as_dir$ac_prog$ac_exec_ext" --version 2>&1` in #(
             'mkdir (GNU coreutils) '* | \
             'mkdir (coreutils) '* | \
             'mkdir (fileutils) '4.1*)
-              ac_cv_path_mkdir=$as_dir/$ac_prog$ac_exec_ext
+              ac_cv_path_mkdir=$as_dir$ac_prog$ac_exec_ext
               break 3;;
           esac
         done
diff --git a/lib/autotest/general.m4 b/lib/autotest/general.m4
index 60c0352..264a01a 100644
--- a/lib/autotest/general.m4
+++ b/lib/autotest/general.m4
@@ -1007,8 +1007,8 @@ do
   case $at_program in
     [[\\/]* | ?:[\\/]* ) $at_program_=$at_program ;;]
     * )
-    _AS_PATH_WALK([$PATH], [test -f "$as_dir/$at_program" && break])
-    at_program_=$as_dir/$at_program ;;
+    _AS_PATH_WALK([$PATH], [test -f "$as_dir$at_program" && break])
+    at_program_=$as_dir$at_program ;;
   esac
   if test -f "$at_program_"; then
     {
diff --git a/lib/m4sugar/m4sh.m4 b/lib/m4sugar/m4sh.m4
index f05346b..19ee12f 100644
--- a/lib/m4sugar/m4sh.m4
+++ b/lib/m4sugar/m4sh.m4
@@ -231,7 +231,7 @@ dnl Remove any tests from suggested that are also required
         /*)
           for as_base in sh bash ksh sh5; do
             # Try only shells that exist, to save several forks.
-            as_shell=$as_dir/$as_base
+            as_shell=$as_dir$as_base
             AS_IF([{ test -f "$as_shell" || test -f "$as_shell.exe"; } &&
                    _AS_RUN(["$as_required"], ["$as_shell"])],
                   [CONFIG_SHELL=$as_shell as_have_required=yes
@@ -468,7 +468,7 @@ as_myself=
 case $[0] in @%:@((
   *[[\\/]]* ) as_myself=$[0] ;;
   *) _AS_PATH_WALK([],
-                  [test -r "$as_dir/$[0]" && as_myself=$as_dir/$[0] && break])
+                  [test -r "$as_dir$[0]" && as_myself=$as_dir$[0] && break])
      ;;
 esac
 # We did not find ourselves, most probably we were run as `sh COMMAND'
@@ -1326,8 +1326,8 @@ fi

 # _AS_PATH_WALK([PATH = $PATH], BODY, [IF-NOT-FOUND])
 # ---------------------------------------------------
-# Walk through PATH running BODY for each `as_dir'.  If BODY never does a
-# `break', evaluate IF-NOT-FOUND.
+# Walk through PATH running BODY for each `as_dir', with a trailing slash
+# already present.  If BODY never does a `break', evaluate IF-NOT-FOUND.
 #
 # Still very private as its interface looks quite bad.
 #
@@ -1340,19 +1340,23 @@ m4_defun_init([_AS_PATH_WALK],
 [AS_REQUIRE([_AS_PATH_SEPARATOR_PREPARE])],
 [as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
 m4_ifvaln([$3], [as_found=false])dnl
-m4_bmatch([$1], [[:;]],
+m4_if([$1], m4_translit([[$1]], [:;]),
+[for as_dir in m4_default([$1], [$PATH])],
 [as_dummy="$1"
-for as_dir in $as_dummy],
-[for as_dir in m4_default([$1], [$PATH])])
+for as_dir in $as_dummy])
 do
   IFS=$as_save_IFS
-  test -z "$as_dir" && as_dir=.
+  case $as_dir in #(((
+    '') as_dir=./ ;;
+    */) ;;
+    *) as_dir=$as_dir/ ;;
+  esac
   m4_ifvaln([$3], [as_found=:])dnl
   $2
   m4_ifvaln([$3], [as_found=false])dnl
 done
-m4_ifvaln([$3], [$as_found || { $3; }])dnl
 IFS=$as_save_IFS
+m4_ifvaln([$3], [$as_found || { $3; }])dnl
 ])


-- 
1.7.10.4




reply via email to

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