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

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

[Kővágó, Zoltán]

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)
+{
+    const char *ptr;
+
+    for (ptr = value; *ptr; ++ptr) {
+        if (*ptr == ',') {
+            printf(",,");
+        } else {
+            putchar(*ptr);
+        }
+    }
+}
+
+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;
     }
 }
 
-- 
2.4.5

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

[Markus Armbruster]

"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?

> +{
> +    const char *ptr;
> +
> +    for (ptr = value; *ptr; ++ptr) {
> +        if (*ptr == ',') {
> +            printf(",,");
> +        } else {
> +            putchar(*ptr);
> +        }
> +    }

Slightly simpler:

        if (*ptr == ',') {
            putchar(',');
        }
        putchar(*ptr);

Matter of taste.

> +}
> +
> +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>

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

[Kővágó Zoltán]

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

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

[Markus Armbruster]

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