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

[Markus Armbruster <armbru@redhat.com>]

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

> 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.

Okay.

>>> +{
>>> +    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?

If it's not too much trouble...  Feel free to keep my R-by then.