Re: [libvirt] [patch 2/5] Instantiate comments in ip(6)tables rules

On 09/24/2010 02:22 PM, Stefan Berger wrote: Hmm. 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. 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 eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org