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

On 09/27/2010 12:40 PM, Stefan Berger wrote: At first, I failed to see how ` needs escaping inside '', unless you aren't uniformly using '' like you think you are. Then it hit me - yuck - we aren't uniformly using '' - we really do have to escape ` inside ``, or come up with an alternate solution. That is: ret=`iptables -m comment --comment '`'` is indeed ill-formed. But if you are going to escape `, then you also have to escape \ and $ for the same duration. Yuck again. But fear not - I have a slicker solution: comment='comment with lone '\'', ", `, \, $x, and two spaces' cmd='iptables ...-m comment --comment "$comment"' eval ret=\`"$cmd"\` which at expansion time results in: eval ret=\`"$cmd"\` ret=`iptables ...-m comment --comment "$comment"` iptables ...-m comment --comment \ 'comment with lone '\'', `, ", `, \, $x, and two spaces' That is, rather than trying to pass the comment literally through $cmd (and thus having to carefully escape $cmd for its eventual use inside ``), it might be nicer to stick the comment in an intermediate variable (where you only need to escape ') and make $cmd reference the intermediate variable (where you won't have any problematic uses of ", `, or ' to contend with, and where your only $ is one where you intentionally want variable expansion). This part seems okay - the ` is quoted to protect it from evaluation until after eval has had a chance to collect its arguments. That seems rather long to me. Broken into chunks with C-string escaping eliminated: ' \'\"\'\"\' ' end the current single quoting the 5 literal shell chars '"'"' resume single quoting I'm not following why we need 5 literal shell characters, instead of 1. Back to my observation that this doesn't help for \ or $, the other two characters that need escaping inside ``. It would be so much easier if we could rely on $() instead of ``. Wait a minute - this is only done for Linux hosts, where we are guaranteed a sane shell (it's pretty much just Solaris where /bin/sh is too puny to do $()) - using $() instead of `` would solve a lot of escaping issues. OK, maybe I see why your comment had such a long replacement for ', because you aren't adding any escaping to cmt here. But I still think we can come up with a more elegant solution, by using the intermediate variable. Thanks for forcing me to explain myself - it's an interesting process thinking about this. [Do I even dare mention that at an even higher layer, it might be nicer to avoid /bin/sh in the first place, and instead put effort into my pending virCommand API patches to make it easier to directly invoke all the iptables commands from C?] -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org