On 05/12/2014 01:55 AM, Michael Tokarev wrote: s/rewamp/revamp/ in the subject
> Main change is to allow get_opt_name() to accept
> a set of delimiters (string) instead of a single
> delimiter (char). This way it is easier to search
> for the next (sub)option in an option string, so
> other code using get_opt_name() can be simplified.
>
> Signed-off-by: Michael Tokarev <m...@tls.msk.ru>
> ---
> This is an old patch (from Jun 2013) which was
> sitting in my local git repository since that.
> I rebased it to current master (with 2 trivial
> conflicts resolved).
>
> FWIW.
>
> include/qemu/option.h | 3 +-
> util/qemu-option.c | 136
> +++++++++++++++++++++++--------------------------
> vl.c | 4 +-
> 3 files changed, 67 insertions(+), 76 deletions(-)
>
> @@ -97,24 +95,27 @@ int get_next_param_value(char *buf, int buf_size,
>
> p = *pstr;
> for(;;) {
> - p = get_opt_name(option, sizeof(option), p, '=');
> - if (*p != '=')
> - break;
> - p++;
> - if (!strcmp(tag, option)) {
> - *pstr = get_opt_value(buf, buf_size, p);
> - if (**pstr == ',') {
> - (*pstr)++;
> + if (*p == '\0') {
> + return 0;
> + }
> + p = get_opt_name(option, sizeof(option), p, "=,");
> + if (strcmp(tag, option) == 0) {
> + if (*p == '=') {
> + p = get_opt_value(buf, buf_size, p + 1);
> + } else {
> + buf[0] = '\0';
> }
> - return strlen(buf);
> + *pstr = *p == ',' ? p + 1 : p;
> + return 1;
Does this botch handling of ',,' escaping that allows literal commas as
part of an option value? I suspect there may be some corner cases you
are altering. Based just on a quick read of 'qemu-kvm -help', I see:
-name string1[,process=string2][,debug-threads=on|off]
Which, if I'm interpreting correctly, means that pre-patch I can do:
qemu-kvm -name foo,,bar /dev/null
as a way to start a (pretty pointless) machine with the window title of
"QEMU (foo,bar)". Reading the source code, I also found the implicit
name of string1:
qemu-kvm -name guest=debug-threads= /dev/null
creates the window title "QEMU (debug-threads=)", and:
qemu-kvm -name debug-threads,, /dev/null
creates the window title "QEMU (debug-threads,)". That is, I have a way
of specifying ANY name, including trailing '=' or ',', for my guest.
I did not apply your patch, but off-hand, I'm guessing that your patch
breaks at least one, if not multiple, of these cases. We need unit
tests in place first.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
