On Wed, Mar 14, 2012 at 10:40:41AM -0600, Eric Blake wrote:
On 03/14/2012 09:19 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
To facilitate parsing of argv[] style strings, provide a virStrSplitQuoted API which will split a string on the listed separators, but also allow for quoting with ' or ".
* src/libvirt_private.syms, src/util/util.c, src/util/util.h: Implement virStrSplitQuoted * tests/utiltest.c: Some tests for virStrSplitQuoted --- src/libvirt_private.syms | 1 + src/util/util.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/util.h | 2 + tests/utiltest.c | 89 ++++++++++++++++++++++++++++++++-- 4 files changed, 203 insertions(+), 6 deletions(-)
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 ?
+ +static char * +virStrDupUnescape(const char *src, size_t len) +{ + char *ret; + size_t i, j; + bool escape = false; + + if (VIR_ALLOC_N(ret, len + 1) < 0) + return NULL; + + for (i = 0, j = 0 ; i < len ; i++) { + if (escape) { + escape = false; + ret[j++] = src[i]; + } else if (src[i] == '\\') { + escape = true; + } else { + ret[j++] = src[i]; + }
So this version only strips backslash. Looks okay in isolation.
+/** + * virStrSplitQuoted: + * @src: string to split + * @sep: list of separator characters + * + * Split the string 'src' into chunks, separated by one or more of + * the characters in 'sep'. Allow ' or " to quote strings even if + * they contain 'sep'
No documentation of backslash handling. Are we trying to emulate shell parsing here (where depending on outer, "", or '', \ behaves differently)?
Or are we trying to emulate C string parsing, where \n is translated to newline?
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. SystemD's parser is what I modelled my code on, though it in fact does not unescape anything. eg. if parsing foo "bar \" wizz" eek systemd seems to return foo bar \" wizz eek while I return foo bar " wizz eek
+struct StringSplitData { + const char *src; + const char *sep; + const char **bits; +};
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 :-)
+ const char *bits1[] = { "foo", "bar", NULL }; + DO_TEST_STRING("foo bar", " ", bits1); + DO_TEST_STRING("foo 'bar'", " ", bits1); + DO_TEST_STRING("foo \"bar\"", " ", bits1); + DO_TEST_STRING(" foo \"bar\"", " ", bits1); + DO_TEST_STRING(" foo \"bar\"", " ", bits1); + DO_TEST_STRING(" foo \"bar\"\n ", " \t\n\r", bits1); + + const char *bits2[] = { "foo", "bar wizz", "eek", NULL }; + DO_TEST_STRING("foo 'bar wizz' eek", " ", bits2); + DO_TEST_STRING("foo \"bar wizz\" eek", " ", bits2);
What about "foo bar\ wizz eek", if \ can escape outside of quotes?
Hmm, possibly
+ + const char *bits3[] = { "foo", "'bar' \"wizz\"", "eek", NULL };
It would also be nice to have a literal backslash in the expected results, to prove that we can properly escape them.
Good point.
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. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|