Re: [Qemu-devel] [PATCH] opts: produce valid command line in qemu_opts_print

["Kővágó Zoltán" <dirty.ice.hu@gmail.com>]

2015-07-03 15:01 keltezéssel, Markus Armbruster írta:
> "Kővágó, Zoltán" <dirty.ice.hu@gmail.com> writes:
>
>> This will let us print options in a format that the user would actually
>> write it on the command line (foo=bar,baz=asd,etc=def), without
>> prepending a spurious comma at the beginning of the list, or quoting
>> values unnecessarily.  This patch provides the following changes:
>> * write and id=, if the option has an id
>> * do not print separator before the first element
>> * do not quote string arguments
>> * properly escape commas (,) for QEMU
>>
>> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
>> ---
>>
>> Changes from patch submitted as part of `qapi flattening':
>> * no longer tries to do proper escaping for the shell
>>
>>   block.c            |  2 +-
>>   util/qemu-option.c | 30 +++++++++++++++++++++++++++---
>>   2 files changed, 28 insertions(+), 4 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 7e130cc..78a5304 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -3813,7 +3813,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
>>       }
>>
>>       if (!quiet) {
>> -        printf("Formatting '%s', fmt=%s", filename, fmt);
>> +        printf("Formatting '%s', fmt=%s ", filename, fmt);
>>           qemu_opts_print(opts, " ");
>>           puts("");
>>       }
>> diff --git a/util/qemu-option.c b/util/qemu-option.c
>> index efe9d27..55ef172 100644
>> --- a/util/qemu-option.c
>> +++ b/util/qemu-option.c
>> @@ -730,14 +730,36 @@ void qemu_opts_del(QemuOpts *opts)
>>       g_free(opts);
>>   }
>>
>> -void qemu_opts_print(QemuOpts *opts, const char *sep)
>> +/* print value, escaping any commas in value */
>> +static void escaped_print(const char *value)
>
> Have you searched the tree for existing code doing this?

I didn't found any function like that.

>> +{
>> +    const char *ptr;
>> +
>> +    for (ptr = value; *ptr; ++ptr) {
>> +        if (*ptr == ',') {
>> +            printf(",,");
>> +        } else {
>> +            putchar(*ptr);
>> +        }
>> +    }
>
> Slightly simpler:
>
>          if (*ptr == ',') {
>              putchar(',');
>          }
>          putchar(*ptr);
>
> Matter of taste.

I also like it better that way. Should I send a v2 patch?

>> +}
>> +
>> +void qemu_opts_print(QemuOpts *opts, const char *separator)
>>   {
>>       QemuOpt *opt;
>>       QemuOptDesc *desc = opts->list->desc;
>> +    const char *sep = "";
>> +
>> +    if (opts->id) {
>> +        printf("id=%s", opts->id); /* passed id_wellformed -> no commas */
>> +        sep = separator;
>> +    }
>>
>>       if (desc[0].name == NULL) {
>>           QTAILQ_FOREACH(opt, &opts->head, next) {
>> -            printf("%s%s=\"%s\"", sep, opt->name, opt->str);
>> +            printf("%s%s=", sep, opt->name);
>> +            escaped_print(opt->str);
>> +            sep = separator;
>>           }
>>           return;
>>       }
>> @@ -750,13 +772,15 @@ void qemu_opts_print(QemuOpts *opts, const char *sep)
>>               continue;
>>           }
>>           if (desc->type == QEMU_OPT_STRING) {
>> -            printf("%s%s='%s'", sep, desc->name, value);
>> +            printf("%s%s=", sep, desc->name);
>> +            escaped_print(value);
>>           } else if ((desc->type == QEMU_OPT_SIZE ||
>>                       desc->type == QEMU_OPT_NUMBER) && opt) {
>>               printf("%s%s=%" PRId64, sep, desc->name, opt->value.uint);
>>           } else {
>>               printf("%s%s=%s", sep, desc->name, value);
>>           }
>> +        sep = separator;
>>       }
>>   }
>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>