[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 4/6] tweak to config.status->libtool quoting (correct?)
From: |
Eric Blake |
Subject: |
Re: [PATCH 4/6] tweak to config.status->libtool quoting (correct?) |
Date: |
Fri, 7 Nov 2008 22:41:28 +0000 (UTC) |
User-agent: |
Loom/3.14 (http://gmane.org/) |
Paolo Bonzini <bonzini <at> gnu.org> writes:
> This is a patch I was not sure of when I wrote it, but now I am more
> confident. config.status was looking for quoting only in the first
> word of variables because of a missing "..." in the arguments of $ECHO.
> $ECHO, depending on how it is set (and always in the final patch) will
> not print arguments after the first.
In Autoconf, AS_ECHO (and thus $as_echo) is documented as printing only a
single shell word. You are correct that it is not safe to pass multiple
arguments to $ECHO, and that "" (or other quoting) is needed to make it one
shell word. 'printf %s\\n' actually prints multiple arguments, but that is not
the only plausible definition of as_echo.
>
> The first hunk is needed for the other patches, but the second does the
> same change, and the current code seems equally wrong. Note that not
> applying this patch is a showstopper for the final patch in the series,
> which defines variables like
>
> printf %s\\n "$1"
>
> There, the first word does not need extra quoting, but the others do.
>
> * libltdl/m4libtool.m4 (_LT_OUTPUT_LIBTOOL_COMMANDS_INIT): Double
> quote ECHO-ed variable.
>
> ---
> libltdl/m4/libtool.m4 | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libltdl/m4/libtool.m4 b/libltdl/m4/libtool.m4
> index 34fa517..27c8d70 100644
> --- a/libltdl/m4/libtool.m4
> +++ b/libltdl/m4/libtool.m4
> @@ -520,7 +520,7 @@ compiler='$compiler_DEFAULT'
> # Quote evaled strings.
> for var in lt_decl_all_varnames([[ \
> ]], lt_decl_quote_varnames); do
> - case \`eval \\\\\$ECHO "X\\\\\$\$var"\` in
> + case \`eval \\\\\$ECHO \\\\""X\\\\\$\$var"\\\\"\` in
> *[[\\\\\\\`\\"\\\$]]*)
This might be a bit easier to write (and thus review) if we used AS_ESCAPE
rather than supplying all the \ ourselves (perhaps as an independent patch
prior to this one). As in:
pre:
AS_ESCAPE([
case `eval \\$ECHO "X\\\$$var"` in
...
], [\`$])
post:
AS_ESCAPE([
case `eval \\$ECHO \""X\\\$$var\""` in
...
], [\`$])
or even nested AS_ESCAPE:
AS_ESCAPE([
case `eval AS_ESCAPE(['$ECHO "X$'])$var[]AS_ESCAPE(['"'])` in
...
], [\`$])
At any rate, the patch indeed makes sense - $var contains a variable name,
which may expand with whitespace, so we must supply literal "" as part of the
string being eval'd in order to protect the expansion of $var as a single shell
word (all within the context of `` needing one level of escapes, and a here-doc
needing another).
--
Eric Blake