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(a)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 :|