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

2015-07-03 Thread Kővágó Zoltán 

2015-07-03 15:01 keltezéssel, Markus Armbruster írta:

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
* properly escape commas (,) for QEMU

Signed-off-by: Kővágó, Zoltán dirty.ice...@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 arm...@redhat.com

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

2015-07-03 Thread Markus Armbruster 
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
 * properly escape commas (,) for QEMU

 Signed-off-by: Kővágó, Zoltán dirty.ice...@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 arm...@redhat.com