[PATCH 2/5] docs: mention cost of globbing during variable expansion

[Eric Blake]

* doc/autoconf.texi (Shell Substitutions) <${var=literal}>:
Recommend quoting substitutions that might trigger globbing.
(Limitations of Builtins) <:>: Likewise.
* bin/autoconf.as: Follow our own advice.
* lib/autoconf/functions.m4 (AC_FUNC_SELECT_ARGTYPES): Likewise.
* lib/autoconf/general.m4 (_AC_INIT_PARSE_ARGS): Likewise.
* lib/autoconf/status.m4 (AC_OUTPUT): Likewise.
* lib/autotest/general.m4 (_AT_FINISH): Likewise.
* lib/m4sugar/m4sh.m4 (AS_TMPDIR): Likewise.
* tests/autotest.at (parallel autotest and signal handling):
Likewise.
* tests/c.at (AC_OPENMP and C, AC_OPENMP and C++): Likewise.
* tests/foreign.at (shtool): Likewise.
* tests/fortran.at: Likewise.
* tests/tools.at (autom4te preselections): Likewise.
* tests/torture.at (VPATH): Likewise.

Signed-off-by: Eric Blake <address@hidden>
---

Wow - it's amazing how much code out there does NOT quote
arguments to :.  Hopefully the documentation will give
an example of why it is useful.  Not every variable default
needs to be quoted, if you know that the result should not
contain glob characters, but we might as well set an example.

 ChangeLog                 |   20 ++++++++++++++++++++
 bin/autoconf.as           |    2 +-
 doc/autoconf.texi         |   45 ++++++++++++++++++++++++++++++++++++++-------
 lib/autoconf/functions.m4 |    2 +-
 lib/autoconf/general.m4   |    2 +-
 lib/autoconf/status.m4    |    2 +-
 lib/autotest/general.m4   |    2 +-
 lib/m4sugar/m4sh.m4       |    2 +-
 tests/autotest.at         |    2 +-
 tests/c.at                |    4 ++--
 tests/foreign.at          |    2 +-
 tests/fortran.at          |   28 ++++++++++++++--------------
 tests/tools.at            |    2 +-
 tests/torture.at          |    2 +-
 14 files changed, 84 insertions(+), 33 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 569d7ad..88fcd88 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,25 @@
 2010-08-25  Eric Blake  <address@hidden>

+       docs: mention cost of globbing during variable expansion
+       * doc/autoconf.texi (Shell Substitutions) <${var=literal}>:
+       Recommend quoting substitutions that might trigger globbing.
+       (Limitations of Builtins) <:>: Likewise.
+       * bin/autoconf.as: Follow our own advice.
+       * lib/autoconf/functions.m4 (AC_FUNC_SELECT_ARGTYPES): Likewise.
+       * lib/autoconf/general.m4 (_AC_INIT_PARSE_ARGS): Likewise.
+       * lib/autoconf/status.m4 (AC_OUTPUT): Likewise.
+       * lib/autotest/general.m4 (_AT_FINISH): Likewise.
+       * lib/m4sugar/m4sh.m4 (AS_TMPDIR): Likewise.
+       * tests/autotest.at (parallel autotest and signal handling):
+       Likewise.
+       * tests/c.at (AC_OPENMP and C, AC_OPENMP and C++): Likewise.
+       * tests/foreign.at (shtool): Likewise.
+       * tests/fortran.at: Likewise.
+       * tests/tools.at (autom4te preselections): Likewise.
+       * tests/torture.at (VPATH): Likewise.
+
+2010-08-25  Eric Blake  <address@hidden>
+
        m4sh: fix some namespace safety issues
        * lib/m4sugar/m4sh.m4 (_AS_SHELL_SANITIZE): Avoid problems if
        as_myself is inherited from environment.
diff --git a/bin/autoconf.as b/bin/autoconf.as
index aa7fe6b..dca4606 100644
--- a/bin/autoconf.as
+++ b/bin/autoconf.as
@@ -85,7 +85,7 @@ exit_missing_arg='
 # restore font-lock: '

 # Variables.
-: ${AUTOM4TE='@bindir@/@autom4te-name@'}
+: "${AUTOM4TE='@bindir@/@autom4te-name@'}"
 autom4te_options=
 outfile=
 verbose=false
diff --git a/doc/autoconf.texi b/doc/autoconf.texi
index a20be76..c1e86c0 100644
--- a/doc/autoconf.texi
+++ b/doc/autoconf.texi
@@ -15398,10 +15398,36 @@ Shell Substitutions

 @item address@hidden@address@hidden@}
 @cindex address@hidden@address@hidden@}
-Be sure to quote:
+When using @address@hidden@address@hidden@}} to assign a default value
+to @var{var}, remember that even though the assignment to @var{var} does
+not undergo file name expansion, the result of the variable expansion
+does.  In particular, when using @command{:} followed by unquoted
+variable expansion for the side effect of setting a default value, if
+either @samp{value} or the prior contents of @samp{$var} contains
+globbing characters, the shell has to spend time performing file name
+expansion and field splitting even though those results will not be
+used.  Therefore, it is a good idea to use double quotes when performing
+default initialization.

 @example
-: address@hidden'Some words'@}
+$ time bash -c ': "address@hidden/usr/bin/address@hidden"; echo "$a"'
+/usr/bin/*
+
+real   0m0.005s
+user   0m0.002s
+sys    0m0.003s
+$ time bash -c ': address@hidden/usr/bin/address@hidden; echo "$a"'
+/usr/bin/*
+
+real   0m0.039s
+user   0m0.026s
+sys    0m0.009s
address@hidden example
+
+Use quotes if @var{literal} contains more than one shell word:
+
address@hidden
+: "address@hidden'Some words'@}"
 @end example

 @noindent
@@ -15441,7 +15467,7 @@ Shell Substitutions

 @example
 default="yu,yaa"
-: address@hidden"$default"@}
+: "address@hidden"$default"@}"
 @end example

 @noindent
@@ -15467,7 +15493,7 @@ Shell Substitutions

 @example
 default="a b c"
-: address@hidden"$default"@}
+: "address@hidden"$default"@}"
 for c in $list; do
   echo $c
 done
@@ -15720,7 +15746,7 @@ Assignments
 brace, use:

 @example
-: address@hidden'my literal'@}
+: "address@hidden'my literal'@}"
 @end example

 @item
@@ -15729,7 +15755,7 @@ Assignments
 (i.e., it's not a list), then use:

 @example
-: address@hidden"$default"@}
+: "address@hidden"$default"@}"
 @end example

 @item
@@ -17432,6 +17458,11 @@ Limitations of Builtins
 for @command{true}.
 @end quotation

+Remember that even though @samp{:} ignores its arguments, it still takes
+time to compute those arguments.  It is a good idea to use double quotes
+around any arguments to @samp{:} to avoid time spent in field splitting
+and file name expansion.
+

 @anchor{unset}
 @item @command{unset}
@@ -18259,7 +18290,7 @@ Limitations of Usual Tools
 # Create a temporary directory $tmp in $TMPDIR (default /tmp).
 # Use mktemp if possible; otherwise fall back on mkdir,
 # with $RANDOM to make collisions less likely.
-: address@hidden/address@hidden
+: "address@hidden/address@hidden"
 @{
   tmp=`
     (umask 077 && mktemp -d "$TMPDIR/fooXXXXXX") 2>/dev/null
diff --git a/lib/autoconf/functions.m4 b/lib/autoconf/functions.m4
index d7058c8..7e69d46 100644
--- a/lib/autoconf/functions.m4
+++ b/lib/autoconf/functions.m4
@@ -1453,7 +1453,7 @@ AC_CACHE_CHECK([types of arguments for select],
  done
 done
 # Provide a safe default value.
-: ${ac_cv_func_select_args='int,int *,struct timeval *'}
+: "${ac_cv_func_select_args='int,int *,struct timeval *'}"
 ])
 ac_save_IFS=$IFS; IFS=','
 set dummy `echo "$ac_cv_func_select_args" | sed 's/\*/\*/g'`
diff --git a/lib/autoconf/general.m4 b/lib/autoconf/general.m4
index 99cc326..c6f3980 100644
--- a/lib/autoconf/general.m4
+++ b/lib/autoconf/general.m4
@@ -900,7 +900,7 @@ Try `$[0] --help' for more information])
     AC_MSG_WARN([you should use --build, --host, --target])
     expr "x$ac_option" : "[.*[^-._$as_cr_alnum]]" >/dev/null &&
       AC_MSG_WARN([invalid host type: $ac_option])
-    : ${build_alias=$ac_option} ${host_alias=$ac_option} 
${target_alias=$ac_option}
+    : "${build_alias=$ac_option} ${host_alias=$ac_option} 
${target_alias=$ac_option}"
     ;;

   esac
diff --git a/lib/autoconf/status.m4 b/lib/autoconf/status.m4
index 56190a4..9bd0a50 100644
--- a/lib/autoconf/status.m4
+++ b/lib/autoconf/status.m4
@@ -1271,7 +1271,7 @@ m4_ifdef([_AC_SEEN_CONFIG(HEADERS)], 
[DEFS=-DHAVE_CONFIG_H], [AC_OUTPUT_MAKE_DEF
 dnl Commands to run before creating config.status.
 AC_OUTPUT_COMMANDS_PRE()dnl

-: ${CONFIG_STATUS=./config.status}
+: "${CONFIG_STATUS=./config.status}"
 ac_write_fail=0
 ac_clean_files_save=$ac_clean_files
 ac_clean_files="$ac_clean_files $CONFIG_STATUS"
diff --git a/lib/autotest/general.m4 b/lib/autotest/general.m4
index ef76e63..e639ce6 100644
--- a/lib/autotest/general.m4
+++ b/lib/autotest/general.m4
@@ -847,7 +847,7 @@ do
 done

 # Autoconf <=2.59b set at_top_builddir instead of at_top_build_prefix:
-: ${at_top_build_prefix=$at_top_builddir}
+: "${at_top_build_prefix=$at_top_builddir}"

 # Perform any assignments requested during argument parsing.
 eval "$at_debug_args"
diff --git a/lib/m4sugar/m4sh.m4 b/lib/m4sugar/m4sh.m4
index 15dd80d..6b1c1ea 100644
--- a/lib/m4sugar/m4sh.m4
+++ b/lib/m4sugar/m4sh.m4
@@ -1617,7 +1617,7 @@ m4_define([_AS_LITERAL_HEREDOC_IF_NO], [$2])
 # which name is inspired by PREFIX (should be 2-4 chars max).
 m4_define([AS_TMPDIR],
 [# Create a (secure) tmp directory for tmp files.
-m4_if([$2], [], [: ${TMPDIR=/tmp}])
+m4_if([$2], [], [: "${TMPDIR=/tmp}"])
 {
   as_tmp=`(umask 077 && mktemp -d "m4_default([$2],
     [$TMPDIR])/$1XXXXXX") 2>/dev/null` &&
diff --git a/tests/autotest.at b/tests/autotest.at
index 04524a5..811417b 100644
--- a/tests/autotest.at
+++ b/tests/autotest.at
@@ -1502,7 +1502,7 @@ for signal in 2 15; do
   # AT_CHECK([[grep '[iI]nterrupt[      ]' stderr]], [1])

   # Ditto with `make' in the loop.
-  : ${MAKE=make}
+  : "${MAKE=make}"
   unset MAKEFLAGS
   # Need to eliminate outer TESTSUITEFLAGS here.
   # Need to normalize exit status here: some make implementations
diff --git a/tests/c.at b/tests/c.at
index 8e77233..0e09798 100644
--- a/tests/c.at
+++ b/tests/c.at
@@ -368,7 +368,7 @@ int main ()
 }
 ]])

-: ${MAKE=make}
+: "${MAKE=make}"
 AT_CHECK([env ACLOCAL=true autoreconf -vi], [], [ignore], [ignore])
 AT_CHECK([./configure $configure_options], [], [ignore], [ignore])
 AT_CHECK([$MAKE], [], [ignore], [ignore])
@@ -411,7 +411,7 @@ AT_DATA([foo.cpp],
 }
 ]])

-: ${MAKE=make}
+: "${MAKE=make}"
 AT_CHECK([env ACLOCAL=true autoreconf -vi], [], [ignore], [ignore])
 AT_CHECK([./configure $configure_options], [], [ignore], [ignore])
 AT_CHECK([$MAKE], [], [ignore], [ignore])
diff --git a/tests/foreign.at b/tests/foreign.at
index b223d20..08cb8b1 100644
--- a/tests/foreign.at
+++ b/tests/foreign.at
@@ -122,7 +122,7 @@ copy-shtool:
 : >file1
 : >file2
 chmod +x file1
-: ${MAKE=make}
+: "${MAKE=make}"
 mkdir build-aux inst
 instdir=`pwd`/inst
 AT_CHECK_AUTOCONF
diff --git a/tests/fortran.at b/tests/fortran.at
index c723748..4397e35 100644
--- a/tests/fortran.at
+++ b/tests/fortran.at
@@ -107,7 +107,7 @@ AT_DATA([foo.f],
       end
 ]])

-: ${MAKE=make}
+: "${MAKE=make}"
 AT_CHECK([env ACLOCAL=true autoreconf -vi], [], [ignore], [ignore])
 AT_CHECK_CONFIGURE
 AT_CHECK([$MAKE], [], [ignore], [ignore])
@@ -147,7 +147,7 @@ AT_DATA([foo.f],
       end
 ]])

-: ${MAKE=make}
+: "${MAKE=make}"
 AT_CHECK([env ACLOCAL=true autoreconf -vi], [], [ignore], [ignore])
 AT_CHECK_CONFIGURE
 AT_CHECK([$MAKE], [], [ignore], [ignore])
@@ -240,7 +240,7 @@ int main(int argc, char *argv[])
 AT_CHECK_AUTOCONF
 AT_CHECK_AUTOHEADER
 AT_CHECK_CONFIGURE
-: ${MAKE=make}
+: "${MAKE=make}"
 AT_CHECK([$MAKE], [], [ignore], [ignore])
 dnl AT_CHECK([./cprogram])

@@ -321,7 +321,7 @@ int main (int argc, char *argv[])
 AT_CHECK_AUTOCONF
 AT_CHECK_AUTOHEADER
 AT_CHECK_CONFIGURE
-: ${MAKE=make}
+: "${MAKE=make}"
 AT_CHECK([$MAKE], [], [ignore], [ignore])
 dnl AT_CHECK([./cprogram])

@@ -400,7 +400,7 @@ int F77_MAIN (int argc, char *argv[])
 AT_CHECK_AUTOCONF
 AT_CHECK_AUTOHEADER
 AT_CHECK_CONFIGURE
-: ${MAKE=make}
+: "${MAKE=make}"
 AT_CHECK([$MAKE], [], [ignore], [ignore])
 dnl AT_CHECK([./cprogram], [], [output from C main
 dnl  some output from Fortran sources
@@ -482,7 +482,7 @@ int FC_MAIN (int argc, char *argv[])
 AT_CHECK_AUTOCONF
 AT_CHECK_AUTOHEADER
 AT_CHECK_CONFIGURE
-: ${MAKE=make}
+: "${MAKE=make}"
 AT_CHECK([$MAKE], [], [ignore], [ignore])
 dnl AT_CHECK([./cprogram], [], [output from C main
 dnl  some output from Fortran sources
@@ -559,7 +559,7 @@ int main(int argc, char *argv[])

 AT_CHECK_AUTOCONF
 AT_CHECK_CONFIGURE
-: ${MAKE=make}
+: "${MAKE=make}"
 AT_CHECK([$MAKE], [], [ignore], [ignore])
 dnl AT_CHECK([./cprogram])

@@ -633,7 +633,7 @@ int main(int argc, char *argv[])

 AT_CHECK_AUTOCONF
 AT_CHECK_CONFIGURE
-: ${MAKE=make}
+: "${MAKE=make}"
 AT_CHECK([$MAKE], [], [ignore], [ignore])
 dnl AT_CHECK([./cprogram])
 AT_CLEANUP
@@ -718,7 +718,7 @@ end

 AT_CHECK_AUTOCONF
 AT_CHECK_CONFIGURE
-: ${MAKE=make}
+: "${MAKE=make}"
 AT_CHECK([$MAKE], [], [ignore], [ignore])

 AT_CLEANUP
@@ -754,7 +754,7 @@ end

 AT_CHECK_AUTOCONF
 AT_CHECK_CONFIGURE
-: ${MAKE=make}
+: "${MAKE=make}"
 AT_CHECK([$MAKE], [], [ignore], [ignore])
 dnl AT_CHECK([./prog])

@@ -792,7 +792,7 @@ end

 AT_CHECK_AUTOCONF
 AT_CHECK_CONFIGURE
-: ${MAKE=make}
+: "${MAKE=make}"
 AT_CHECK([$MAKE], [], [ignore], [ignore])
 dnl AT_CHECK([./prog])

@@ -830,7 +830,7 @@ C      fixed-form style comment

 AT_CHECK_AUTOCONF
 AT_CHECK_CONFIGURE
-: ${MAKE=make}
+: "${MAKE=make}"
 AT_CHECK([$MAKE], [], [ignore], [ignore])
 dnl AT_CHECK([./prog])

@@ -869,7 +869,7 @@ C      fixed-form style comment

 AT_CHECK_AUTOCONF
 AT_CHECK_CONFIGURE
-: ${MAKE=make}
+: "${MAKE=make}"
 AT_CHECK([$MAKE], [], [ignore], [ignore])
 dnl AT_CHECK([./prog])

@@ -933,7 +933,7 @@ EOF

     AT_CHECK_AUTOCONF
     AT_CHECK_CONFIGURE
-    : ${MAKE=make}
+    : "${MAKE=make}"
     AT_CHECK([$MAKE], [], [ignore], [ignore])
     dnl AT_CHECK([./prog])
     AT_CHECK([$MAKE clean], [], [ignore], [ignore])
diff --git a/tests/tools.at b/tests/tools.at
index 167d68a..20e9037 100644
--- a/tests/tools.at
+++ b/tests/tools.at
@@ -1160,7 +1160,7 @@ AT_CLEANUP
 # -----------------------------

 AT_SETUP([autom4te preselections])
-: ${sleep='sleep 1'}   # Command to force different time stamps.
+: "${sleep='sleep 1'}" # Command to force different time stamps.
 # If this test should run on FAT file systems and older w32,
 # then setting $sleep correctly needs to be revisited.

diff --git a/tests/torture.at b/tests/torture.at
index 97cb5c6..a8a2aa1 100644
--- a/tests/torture.at
+++ b/tests/torture.at
@@ -1213,7 +1213,7 @@ all: f f1 f2

 AT_CHECK_AUTOCONF

-: ${MAKE=make}
+: "${MAKE=make}"

 # In place.
 AT_CHECK([./configure $configure_options], [], [ignore])
-- 
1.7.2.2