[5/5] opts: produce valid command line in qemu_opts_print

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 which only contains letters or numbers
* properly escape commas (,) for QEMU, apostrophe (') for shell

Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>

---

Chages from v1 -audiodev patch:
* print id=
* proper value escaping (apostrophe and comma)
* renamed d_sep -> separator


 block.c            |  2 +-
 util/qemu-option.c | 47 ++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 45 insertions(+), 4 deletions(-)

"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 which only contains letters or numbers
> * properly escape commas (,) for QEMU, apostrophe (') for shell
>
> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
>
> ---
>
> Chages from v1 -audiodev patch:
> * print id=
> * proper value escaping (apostrophe and comma)
> * renamed d_sep -> separator
>
>
>  block.c            |  2 +-
>  util/qemu-option.c | 47 ++++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 45 insertions(+), 4 deletions(-)
>
> diff --git a/block.c b/block.c
> index e2e33fd..a18af00 100644
> --- a/block.c
> +++ b/block.c
> @@ -3825,7 +3825,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..db60ac9 100644
> --- a/util/qemu-option.c
> +++ b/util/qemu-option.c
> @@ -730,14 +730,53 @@ void qemu_opts_del(QemuOpts *opts)
>      g_free(opts);
>  }
>  
> -void qemu_opts_print(QemuOpts *opts, const char *sep)
> +/* print value properly escaping it for the shell (at least for bash) */
> +static void escaped_print(const char *value)
> +{
> +    const char *ptr;
> +    bool need_quote = false;
> +
> +    for (ptr = value; *ptr; ++ptr) {
> +        if (!qemu_isalnum(*ptr)) {
> +            need_quote = true;
> +            break;
> +        }
> +    }

Rather eager to quote.  Shell wants you to quote

    |  &  ;  <  >  (  )  $ `  \  "  '  <space>  <tab>  <newline>

always, and

    *   ?   [   #   ˜   =   %

in some contexts.

In particular, there is no need to quote the common characters '.', '-'
and '_'.

http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_02

> +
> +    if (need_quote) {
> +        putchar('\'');
> +        for (ptr = value; *ptr; ++ptr) {
> +            if (*ptr == '\'') {
> +                printf("'\\''");

Won't achieve the stated purpose "for shell", because backslash escapes
do not work within single quotes.

You could try double quote, but things become even more complicated
then.

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

If we do backslash escapes, should we escape non-printing characters?

> +        }
> +        putchar('\'');
> +    } else {
> +        printf("%s", value);
> +    }
> +}
> +
> +void qemu_opts_print(QemuOpts *opts, const char *separator)
>  {
>      QemuOpt *opt;
>      QemuOptDesc *desc = opts->list->desc;
> +    const char *sep = "";
> +
> +    if (opts->id) {
> +        printf("id=");
> +        escaped_print(opts->id);

Since opts->id has passed id_wellformed(), it won't need quoting.
Your escaped_print() may quote it anyway, though.

> +        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 +789,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;
>      }
>  }

Things I like about the patch:

* Making parameter sep a true separator

* Printing opts->id

* Escaping comma

I'm not sure the "quote for shell" feature is worth the bother.  If we
want it, then

    "foo=<bar>,msg=\"hello\",gnu=on"

would perhaps be nicer than

    foo="<bar>",msg="\"hello\"",gnu=on

but could be even more complicated to do.

If you want to pursue quoting for shell, I recommend to split this patch
into a part I like and a part we want to discuss :)