On 09/24/2010 02:22 PM, Stefan Berger wrote:
I just tried the TCK test without and with double-escaping in libvirtd
and double-escaping does seem to be necessary otherwise `ls` and $(ls)
do get executed and their results end up in the comment. The spaces
are preserved, though, so I can revert the change to IFS.
Hmm.
"res=`eval \"$cmd\"" CMD_SEPARATOR
+ virBufferVSprintf(buf,
+ " -m comment --comment \\\"%s\\\"",
+ cmt);
Thinking about it more:
Let's see why double-escaping helped you, by setting up a user comment
with two spaces and a $. You ultimately want to call:
iptables -m comment --comment 'user $comment'
but you used a "" style, resulting in:
iptables -m comment --comment "user \$comment"
Then, you are capturing that command in a shell variable $cmd (to avoid
repetition when displaying a failed command) and the output in $res. So
you need one level of escaping for assigning to cmd within "" context:
cmd="iptables -m comment --comment \"user \\\$comment\""
res=`$cmd`
Oops, that failed, because it splits $cmd at IFS boundaries, which
breaks up the user comment because it looks roughly like
res=`'iptables' '-m' 'comment' '--comment' '"user' '\$comment"'`
But quoting $cmd isn't right either, because then you try to execute a
single command with no arguments:
res=`"$cmd"`
res=`'iptables -m comment --comment "user \$comment"'`
So you _do_ need eval to control where the word boundaries are, which
means you will be removing a level of quoting from the contents of $cmd,
and you also need quoting when assigning to cmd, so I now see why double
escaping is needed. Continuing on with your original example, your
underquoted version with IFS cleared to avoid field splitting of $cmd
looks like:
IFS=
cmd="iptables -m comment --comment \"user \\\$comment\""
res=`eval $cmd`
res=`eval 'iptables -m comment --comment "user \$comment"'`
res=`iptables -m comment --comment "user $comment"`
oops - here, `` turned \$ into $ before the eval got run, which means
eval will expand the (empty variable) $comment. Even with two levels of
quoting, you risk problems when mixing "" and ``.
My suggestion is to assign cmd using '' rather than "" (fewer things to
quote), as well as moving the eval outside of the `` (so it becomes
obvious which \ are interpreted by eval rather than by ``:
cmd='iptables -m comment --comment '\''user $comment'\''
eval res=\`"$cmd"\`
res=`iptables -m comment --comment 'user $comment'`
And the nice part of that is the implementation:
virBufferVSprintf(buf, " -m comment --comment '%s'",
escapeSingleQuotes(user_comment));
virBufferVSprintf(cmd, "cmd='%s'\nres=\\`\"$cmd\"\\`",
escapeSingleQuotes(buf));
still results in the needed double quoting (one for assignment to cmd,
and one to be stripped by eval), but via two passes of a single function
that only has to escape all instances of ', rather than a function that
has to add escaping for four different bytes and pay attention to how
many escape levels must be added.
Ouch. That's probably 4x slower than the glibc version. I'd much
rather see:
#undef strchr
Yes, that does the trick. Thanks.
or
(strchr)(a, b)
On further thought, gnulib might be doing:
#define strchr rpl_strchr
on platforms where strchr is broken, so using #undef strchr is too
risky. So I'd recommend sticking with (strchr)(a, b), which still works
if gnulib has to replace a broken strchr.
(It's surprising how easy it is to have bugs in such a low-level
function - until just this month, glibc 2.12 on Alpha hardware had a bug
in memchr that gnulib has to work around).
--
Eric Blake ebl...@redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
