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

[Gary V. Vaughan]

Hi Eric,

On 26 Aug 2010, at 06:18, Eric Blake wrote:
> * 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.


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

Really?  Not here:

; bash --version
GNU bash, version 3.2.48(1)-release (x86_64-apple-darwin10.0)
Copyright (C) 2007 Free Software Foundation, Inc.
; time bash -c ': "${var=usr/bin/*}"; echo $var'
usr/bin/*

real    0m0.004s
user    0m0.001s
sys     0m0.003s
; time bash -c ': ${var=usr/bin/*}; echo $var'
usr/bin/*

real    0m0.004s
user    0m0.001s
sys     0m0.003s

Does this only affect old/brain-damaged shells?

> +Use quotes if @var{literal} contains more than one shell word:
> +
> address@hidden
> +: "address@hidden'Some words'@}"
> @end example

Still not sold on the outer double quotes...

> @noindent
> @@ -15441,7 +15467,7 @@ Shell Substitutions
> 
> @example
> default="yu,yaa"
> -: address@hidden"$default"@}
> +: "address@hidden"$default"@}"
> @end example

I don't understand this either, since it is tokenised like this:

  :
  ${var=
  $default     => expanded, globbed and split
  }

For example:

; default="yu   *     ya"
; : "${var="$default"}"; echo $var
yu Desktop Documents Downloads Library Movies Music Pictures Public Sites bin ya

Where earlier you say that $default should be quoted.  I've reread
the newly edited section of the manual through twice, and while I
appreciate that you're trying to give an example of triggering a
shell bug on Solaris and/or OSF... the "nested" double quotes in
the example look extremely weird - I don't think anyone would
accidentally or otherwise quote the ${var= and } token but leave
the variable expansion unquoted.  Or are you trying to show a
situation where that is necessary to avoid bugs?  Maybe the manual
is just confusing in this section, and I misunderstand?


> @@ -15720,7 +15746,7 @@ Assignments
> brace, use:
> 
> @example
> -: address@hidden'my literal'@}
> +: "address@hidden'my literal'@}"
> @end example

Agreed - although the outer double quotes seem superfluous to me.

> @item
> @@ -15729,7 +15755,7 @@ Assignments
> (i.e., it's not a list), then use:
> 
> @example
> -: address@hidden"$default"@}
> +: "address@hidden"$default"@}"
> @end example

As long as you don't care about whitespace compression and globbing...
Didn't you mean to say:

: "address@hidden@}"

Otherwise you get bitten by:

default="yu      *   yaa"

again.

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

While that may be strictly true, it seems that bash (at least) doesn't
show any difference in speed regardless.

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

Surely, with a literal containing no spaces, glob meta chars or
closing braces, scanning the extra two characters is actually
infinitesimally slower, but otherwise functionally identical?

> --- 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}"
>     ;;

Agreed - although I prefer the look of:

  : ${build_alias="$ac_option"} ${host_alias="$ac_option"} 
${target_alias="$ac_option"}

But that's just a stylistic issue, unless one form avoids a shell bug somewhere?

[[snip many more examples of the same]]

Cheers,
-- 
Gary V. Vaughan (address@hidden)

PGP.sig
Description: This is a digitally signed message part