[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Add a virStrSplitQuoted for splitting quoted strings



On 03/14/2012 10:50 AM, Daniel P. Berrange wrote:
>> This looks like it is repeating some of the code in
>> virsh.c:vshCommandStringGetArg; any chance we can combine them?  In
>> particular, the ability to mimic shell handling of \ escapes, as well as
>> the difference in behavior of \ inside "" vs. '', seems like it will
>> come in handy.
> 
> Ah I had forgotten about that code. Can you clarify the difference
> in \ handling.  I guess you mean that inside '', \ can only be used
> for  \\ and \', while inside "", it can do all the standard shell
> escapes like \t, \n, etc ?

Inside '', \ has no special meaning. (There's no way to escape ' inside
of ').

Inside "", \ escapes the next byte (important for " and \, but works on
any byte.

Outside quotes, \ escapes the next byte.

And most importantly: concatenating two forms of quoted materials is
permitted.  Thus:

foo"b'ar\" "\'' blah\'

on input becomes this on output:

foob'ar" ' blah\

>>
>> What you have here does neither; although I didn't spot any flaw in the
>> code, I don't know if it's the algorithm we want to be using.
> 
> I should have sent this paired with my other patch for <cmdline>
> handling in LXC. That is the intended use case for this function.
> 
> I'm not sure that anyone has ever clearly defined what escaping
> syntax is used for /proc/cmdline (which is what <cmdline> is
> representing.

Hmm, good point.  If it is /bin/sh doing the parsing, then we know the
rules (and virsh matches them); but it if is a custom parser in the
kernel, the we have to match that custom parser.

>> A point in your favor, for at least testing what you parse!  If we
>> change our mind to mimic shell or C parsing, then we'd have to update
>> these tests.
> 
> Yes, escaping rules blow my mind unless I can test them :-)

You're not the only one - I added loads of tests when I added shell
parsing to virsh.

>> Overall, I like the idea of the new function, but I'm worried that
>> introducing yet another parser could hurt us (users will be asking "now
>> which escape style is in effect here, and how does it differ from
>> standardized escape styles that I'm used to?").
> 
> I think that perhaps we should have  virStrSplitQuoted just return
> the split pieces, with *no* unescaping. And then have separate
> functions to escape/unescape individual string pieces after the
> fact.

Except that you then have to know what quoting styles were in use to
know whether \ was a literal or an escape.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]