Re: interrupt causes parse error in configure script

[Eric Blake]

Ralf Wildenhues <Ralf.Wildenhues <at> gmx.de> writes:

> > $ sh -c 'if test `sleep 5; echo hi` = hi ; then echo yes; fi'
> 
> and found only pdksh and OpenBSD sh (which is a pdksh descendant) to
> have this issue.

Jim's original error message does not look the same as pdksh; it reminds me 
more of what bash would output.  I could not reproduce with bash 3.2.39, but I 
know that at least bash 3.2.20 introduced some fixes for poor subshell behavior:

ftp://ftp.gnu.org/pub/gnu/bash/bash-3.2-patches/bash32-020

Jim, which shell did you see this on?

> 
> What do you think about this patch (putting Eric and me in ChangeLog as
> co authors)?

We're getting closer.

> 
> FWIW I'm still looking into a couple of test failures (one of "AS_IF and
> AS_CASE" which I don't think this patch caused).

I know that when testing my m4_transform_pair patch, I was able to trip an 
arbitrary limit in bash - it refuses to parse more than 2500 nested elif 
keywords inside an if/else/fi.  I wonder if we are tripping some other shell 
limits with my test enhancements last week, even though I scaled back the limit 
to 1000 instead of 2500.  Maybe it was a mistake to add torture tests to the 
already existing AS_IF test, instead of creating a new test?

> 
> I suppose the AS_VAR* macros deserve documentation eventually.

Yes, they are probably stable enough to document, especially if we decide to 
keep the line in your patch that references them.

> Prompted by this (interrupting a pdksh configure run):

Are we sure this is just pdksh?

> 
> +Upon interrupt, pdksh and OpenBSD sh may abort a command substitution,
> +replace it with a null string, and evaluate the enclosing command before
> +interrupting the script.  This can lead to spurious errors:
> +
> address@hidden
> +$ @kbd{pdksh -c 'if test `sleep 5; echo hi` = hi ; then echo yes; fi'}
> +$ @kbd{^C}
> +pdksh: test: hi: unexpected operator/operand
> address@hidden example
> +
> address@hidden
> address@hidden can be used to avoid this.

It might be nice to also show the solution without AS_VAR_IF - namely, the use 
of a temporary variable to split the subshell away from the test, since not all 
readers of this section are writing shell code for autoconf.  Also, I did 
verify that external commands are also incorrectly run, so it is not just the 
test builtin that suffer from the aborted ``:

$ pdksh -c 'if /bin/test `sleep 5; echo hi` = hi ; then echo yes; fi'
^C
/bin/test: missing argument after `hi'

For that matter, AS_VAR_IF doesn't solve everything:

$ pdksh -c 'sleep `sleep 5` 5'
^C

sleeps for 5 seconds beyond the ^C; but has nothing to do with test.

And I guess this means we need to audit autoconf for any other uses of `` that 
are passed directly as arguments to enclosing commands.

>  
> +# AS_VAR_IF(VARIABLE, [VALUE = yes], IF-TRUE, IF-FALSE)

Hmm.  All the rewrites in the patch supplied an explicit [yes], rather than 
relying on [] as the second argument.  Theoretically, it might be nice to check 
that a variable is indeed empty, although that could still be done by passing 
[[]], so it is still safe to let [] default to yes.

> +# -----------------------------------------------------
> +# Implement a shell `if test $VARIABLE = VALUE; then-else'.
> +# Polymorphic, and avoids pdksh expansion error upon interrupt.
> +m4_define([AS_VAR_IF],
> +[AS_LITERAL_IF([$1],
> +  [AS_IF([test "x$$1" = x""m4_default([$2], [yes])], [$3], [$4])],
> +  [as_val=AS_VAR_GET([$1])
> +   AS_IF([test "x$as_val" = x""m4_default([$2], [yes])], [$3], [$4])])])

Now we're getting back to Ralf's first complaint, that by adding "", we lose 
the ability to catch logic bugs when comparing against safe strings (ie. if one 
of multiple branches failed to set the var, perhaps because of a typo).  I'm 
wondering if we should make this smarter, by adding some m4 magic to output 
this:

test "x$as_val" = x""$2

when $2 contains any shell metacharacters or a leading character that might 
mess up test (note that you would thus use AC_VAR_IF([var], ["yes"], ...) if 
you suspect that $var might be "guessing no"); while generating:

test $as_val = m4_default([$2], [yes])

when $2 is [] or purely safe characters.  In other words, if we are comparing 
against a simple string, then let's keep in that safety net that validates that 
$1 was indeed a variable with a known safe value.

-- 
Eric Blake