Re: patch for problems with echo '-...' and echo '...\...'

[Ralf Wildenhues]

Hello Paul,

* Paul Eggert wrote on Fri, Nov 17, 2006 at 10:10:56PM CET:
> I fixed the problems that Ralf spotted in my pre-2.61 proposal (at
> least, the problems I understood... :-)

I guess you meant the ones that you didn't think were bogus.  ;-)
But point taken: I wasn't to the point enough.  Hope this is clearer:

$ x=`perl -e 'print "x"x2060 . "\n"'`
$ /bin/printf %s\\n $x
Bus Error - core dumped
$ config.guess
sparc-sun-solaris2.7

Larger values generate a segmentation fault.  And no, even ignoring all
the real-world installations and the real-world bug reports for even
older Solaris versions, EOSL for 2.7 is another couple of years away:
http://www.sun.com/service/eosl/solaris/solaris_vintage_eol_5.2005.xml

Libtool chooses /usr/ucb/echo here, BTW.  That echo seems to work fine
up to the maximum command line length available on the system.

Interestingly, the `printf %65536s x' Jim just added to coreutils tests
seems to work on the system.

More (incomplete) nits inline.

> --- bin/Makefile.am   20 Sep 2006 02:44:51 -0000      1.26
> +++ bin/Makefile.am   17 Nov 2006 21:02:02 -0000
> @@ -65,7 +65,9 @@
>  ## Use chmod -w to prevent people from editing the wrong file by accident.
>  $(bin_SCRIPTS): Makefile
>       rm -f $@ address@hidden
> -     $(edit) `test -f ./address@hidden || echo $(srcdir)/address@hidden 
> >address@hidden
> +     srcdir=''; \
> +       test -f ./address@hidden || srcdir=$(srcdir)/; \
> +       $(edit) address@hidden >address@hidden
>       chmod +x address@hidden
>       chmod a-w address@hidden
>       mv address@hidden $@

This doesn't help backslashes, which I assume was the major problem spot
here -- white space in $(srcdir) isn't possible with portable make.  But
Automake anyway doesn't cope well with backslashes either.  OTOH, your
change doesn't look like it would hurt much; maybe it even saves an exec.
:-)

> --- bin/autoconf.as   24 Oct 2006 13:13:29 -0000      1.22
> +++ bin/autoconf.as   17 Nov 2006 21:02:02 -0000
> @@ -71,16 +71,17 @@
>  help="\
>  Try \`$as_me --help' for more information."
> 
> -exit_missing_arg="\
> -echo \"$as_me: option \\\`\$1' requires an argument\" >&2
> -echo \"\$help\" >&2
> -exit 1"
> +exit_missing_arg='
> +  AS_ECHO(["$as_me: option \`$[1]'\'' requires an argument"]) >&2
> +  AS_ECHO(["$help"]) >&2
> +  exit 1
> +'

Why do these need AS_ECHO?  More generally, what was the rationale for
changing some but not all instances?

> @@ -92,10 +93,10 @@
>      --version | -V )
>         echo "$version" ; exit ;;
>      --help | -h )
> -       echo "$usage"; exit ;;
> +       AS_ECHO(["$usage"]); exit ;;

This one will break on Solaris 2.7, when a few (3 or 4) more options
are added to autoconf.  OTOH I don't see the necessity for it (no
dangerous backslashes, no leading hyphen in $usage).

> @@ -139,8 +140,8 @@
>         break ;;
>      -* )
>         exec >&2
> -       echo "$as_me: invalid option $1"
> -       echo "$help"
> +       AS_ECHO(["$as_me: invalid option $[1]"])
> +       AS_ECHO(["$help"])

Superfluous IMHO, as some other instances in this file.

> --- lib/autoconf/fortran.m4   20 Sep 2006 18:07:47 -0000      1.213
> +++ lib/autoconf/fortran.m4   17 Nov 2006 21:02:02 -0000

> @@ -535,7 +540,7 @@
>  shift
>  _AS_ECHO_LOG([$[*]])
>  ac_[]_AC_LANG_ABBREV[]_v_output=`eval $ac_link AS_MESSAGE_LOG_FD>&1 2>&1 | 
> grep -v 'Driving:'`
> -echo "$ac_[]_AC_LANG_ABBREV[]_v_output" >&AS_MESSAGE_LOG_FD
> +AS_ECHO(["$ac_[]_AC_LANG_ABBREV[]_v_output"]) >&AS_MESSAGE_LOG_FD
>  _AC_LANG_PREFIX[]FLAGS=$ac_save_FFLAGS
> 
>  rm -f conftest*

Another potentially dangerous change (two instances).

> --- lib/autoconf/status.m4    13 Sep 2006 04:48:23 -0000      1.118
> +++ lib/autoconf/status.m4    17 Nov 2006 21:02:03 -0000

> @@ -1323,10 +1323,12 @@
>  dnl Check this before opening the log, to avoid a bug on MinGW,
>  dnl which prohibits the recursive instance from truncating an open log.
>  if \$ac_cs_recheck; then
> -  echo "running CONFIG_SHELL=$SHELL $SHELL $[0] "$ac_configure_args 
> \$ac_configure_extra_args " --no-create --no-recursion" >&AS_MESSAGE_FD
> -  CONFIG_SHELL=$SHELL
> +  set X '$SHELL' '$[0]'$ac_configure_args \$ac_configure_extra_args 
> --no-create --no-recursion
> +  shift
> +  \AS_ECHO(["running CONFIG_SHELL=$SHELL \$[*]"]) >&AS_MESSAGE_FD
> +  CONFIG_SHELL='$SHELL'
>    export CONFIG_SHELL
> -  exec $SHELL "$[0]"$ac_configure_args \$ac_configure_extra_args --no-create 
> --no-recursion
> +  exec "address@hidden"
>  fi
> 
>  _ACEOF
> @@ -1337,7 +1339,7 @@
>  {
>    echo
>    AS_BOX([Running $as_me.])
> -  echo "$ac_log"
> +  AS_ECHO(["$ac_log"])
>  } >&AS_MESSAGE_LOG_FD
> 
>  _ACEOF

These two hunks are more candidates for overflow of the broken Solaris
printf.

> --- lib/autotest/general.m4   27 Oct 2006 23:05:34 -0000      1.216
> +++ lib/autotest/general.m4   17 Nov 2006 21:02:03 -0000

> @@ -234,9 +233,9 @@
>  at_debug_args=
>  # -e sets to true
>  at_errexit_p=false
> -# Shall we be verbose?
> +# Shall we be verbose?  ':' means no, empty means yes.
>  at_verbose=:
> -at_quiet=echo
> +at_quiet=
> 
>  # Shall we keep the debug scripts?  Must be `:' when the suite is
>  # run by a debug script, so that the script doesn't remove itself.
> @@ -328,7 +327,7 @@
>       ;;
> 
>      --verbose | -v )
> -     at_verbose=echo; at_quiet=:
> +     at_verbose=; at_quiet=:

Erm, using `:' aka. `true' for meaning something to be false seems to
me rather, umm, unintuitive.  Not sure if switching names of at_verbose
and at_quiet would help here.

> @@ -342,14 +341,14 @@
>      # Ranges
>      [[0-9]- | [0-9][0-9]- | [0-9][0-9][0-9]- | [0-9][0-9][0-9][0-9]-])
>       at_range_start=`echo $at_option |tr -d X-`
> -     at_range=`echo " $at_groups_all " | \
> +     at_range=`AS_ECHO([" $at_groups_all "]) | \
>         sed -e 's/^.* \('$at_range_start' \)/\1/'`
>       at_groups="$at_groups$at_range "
>       ;;
> 
>      [-[0-9] | -[0-9][0-9] | -[0-9][0-9][0-9] | -[0-9][0-9][0-9][0-9]])
>       at_range_end=`echo $at_option |tr -d X-`
> -     at_range=`echo " $at_groups_all " | \
> +     at_range=`AS_ECHO([" $at_groups_all "]) | \
>         sed -e 's/\( '$at_range_end'\) .*$/\1/'`
>       at_groups="$at_groups$at_range "
>       ;;
> @@ -367,7 +366,7 @@
>         at_range_end=$at_range_start
>         at_range_start=$at_tmp
>       fi
> -     at_range=`echo " $at_groups_all " | \
> +     at_range=`AS_ECHO([" $at_groups_all "]) | \
>         sed -e 's/^.*\( '$at_range_start' \)/\1/' \
>             -e 's/\( '$at_range_end'\) .*$/\1/'`
>       at_groups="$at_groups$at_range "
> @@ -394,12 +393,14 @@
>           ;;
>         esac
>         # It is on purpose that we match the test group titles too.
> -       at_groups_selected=`echo "$at_groups_selected" |
> +       at_groups_selected=`AS_ECHO(["$at_groups_selected"]) |
>             grep -i $at_invert ["^[1-9][^;]*;.*[; ]$at_keyword[ ;]"]`
>       done
> -     at_groups_selected=`echo "$at_groups_selected" | sed 's/;.*//'`
>       # Smash the newlines.
> -     at_groups="$at_groups`echo $at_groups_selected` "
> +     at_groups_selected=`AS_ECHO(["$at_groups_selected"]) | sed 's/;.*//' |
> +       tr "$as_nl" ' '
> +     `
> +     at_groups="$at_groups$at_groups_selected "
>       ;;
>  m4_divert_pop([PARSE_ARGS])dnl
>  dnl Process *=* last to allow for user specified --option=* type arguments.

The above are all candidates for Solaris printf overflow.

> @@ -429,10 +430,10 @@
>    at_groups=$at_groups_all
>  else
>    # Sort the tests, removing duplicates:
> -  at_groups=`echo $at_groups | tr ' ' "$as_nl" | sort -nu`
> +  at_groups=`AS_ECHO(["$at_groups"]) | tr ' ' "$as_nl" | sort -nu`
>    # and add banners.  (Passing at_groups_all is tricky--see the comment
>    # starting with "Passing at_groups is tricky.")
> -  at_groups=`echo "$at_groups$as_nl $at_groups_all" |
> +  at_groups=`AS_ECHO(["$at_groups$as_nl $at_groups_all"]) |
>      awk ['BEGIN { FS = "@" } # Effectively switch off field splitting.
>       /^$/ { next }  # Ignore the empty line.
>       !/ / { groups++; selected[$ 0] = 1; next }
> @@ -522,7 +523,7 @@
>    # Passing at_groups is tricky.  We cannot use it to form a literal string
>    # or regexp because of the limitation of AIX awk.  And Solaris' awk
>    # doesn't grok more than 99 fields in a record, so we have to use `split'.
> -  echo "$at_groups$as_nl$at_help_all" |
> +  AS_ECHO(["$at_groups$as_nl$at_help_all"]) |
>      awk 'BEGIN { FS = ";" }
>        NR == 1 {
>          for (n = split($ 0, a, " "); n; n--) selected[[a[n]]] = 1

Likewise.

> @@ -943,11 +944,11 @@
>  esac
> 
>  if test $at_unexpected_count = 0; then
> -  echo "$at_result"
> -  echo "$at_result" >&AS_MESSAGE_LOG_FD
> +  AS_ECHO(["$at_result"])
> +  AS_ECHO(["$at_result"]) >&AS_MESSAGE_LOG_FD
>  else
> -  echo "ERROR: $at_result" >&2
> -  echo "ERROR: $at_result" >&AS_MESSAGE_LOG_FD
> +  AS_ECHO(["ERROR: $at_result"]) >&2
> +  AS_ECHO(["ERROR: $at_result"]) >&AS_MESSAGE_LOG_FD
>    {
>      echo
>      AS_BOX([Summary of the failures.])

These can remain plain echo.

> @@ -1419,7 +1420,7 @@
>  ))dnl
>  dnl
>  m4_ifval(m4_defn([at_reason]),
> -[echo 'Not enabling shell tracing (command contains 
> ]m4_defn([at_reason])[)'],
> +[AS_ECHO(['Not enabling shell tracing (command contains 
> ]m4_defn([at_reason])[)'])],
>  [m4_bmatch([$1], [\$],
>  dnl COMMANDS may contain parameter expansions; expand them at runtime.
>  [case "AS_ESCAPE([$1], [`"\])" in

Likewise.

> --- lib/m4sugar/m4sh.m4       15 Oct 2006 01:12:02 -0000      1.201
> +++ lib/m4sugar/m4sh.m4       17 Nov 2006 21:02:03 -0000

> @@ -1418,12 +1464,13 @@
>  # Get the value of the shell VARIABLE.
>  # Evaluates to $VARIABLE if there are no indirection in VARIABLE,
>  # else into the appropriate `eval' sequence.
> -# FIXME: This mishandles values that end in newlines, or have backslashes,
> -# or are '-n'.  Fixing this will require changing the API.
> +# FIXME: This mishandles values that end in newlines.
> +# Fixing this will require changing the API.
>  m4_define([AS_VAR_GET],
>  [AS_LITERAL_IF([$1],
>              [$$1],
> -            [`eval echo '${'m4_bpatsubst($1, [[\\`]], [\\\&])'}'`])])
> +            [`eval 'as_val=${'m4_bpatsubst([$1], [[\\`]], [\\\&])'}
> +              AS_ECHO(["$as_val"])'`])])
> 
> 
>  # AS_VAR_TEST_SET(VARIABLE)

This should have exposure in the testsuite, so the claim is a founded
one.

Hope that helps.

Cheers,
Ralf