"Kővágó, Zoltán" <dirty.ice...@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...@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 :)
