qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: qemu-devel@nongnu.org
Cc: kwolf@redhat.com, qemu-block@nongnu.org, mreitz@redhat.com
Subject: [PATCH for-5.1 8/8] qemu-option: Move is_valid_option_list() to qemu-img.c and rewrite
Date: Thu,  9 Apr 2020 17:30:41 +0200	[thread overview]
Message-ID: <20200409153041.17576-9-armbru@redhat.com> (raw)
In-Reply-To: <20200409153041.17576-1-armbru@redhat.com>

is_valid_option_list()'s purpose is ensuring qemu-img.c's can safely
join multiple parameter strings separated by ',' like this:

        g_strdup_printf("%s,%s", params1, params2);

How it does that is anything but obvious.  A close reading of the code
reveals that it fails exactly when its argument starts with ',' or
ends with an odd number of ','.  Makes sense, actually, because when
the argument starts with ',', a separating ',' preceding it would get
escaped, and when it ends with an odd number of ',', a separating ','
following it would get escaped.

Move it to qemu-img.c and rewrite it the obvious way.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qemu/option.h |  1 -
 qemu-img.c            | 26 ++++++++++++++++++++++++++
 util/qemu-option.c    | 22 ----------------------
 3 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/include/qemu/option.h b/include/qemu/option.h
index 844587cab3..eb4097889d 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -33,7 +33,6 @@ const char *get_opt_value(const char *p, char **value);
 void parse_option_size(const char *name, const char *value,
                        uint64_t *ret, Error **errp);
 bool has_help_option(const char *param);
-bool is_valid_option_list(const char *param);
 
 enum QemuOptType {
     QEMU_OPT_STRING = 0,  /* no parsing (use string as-is)                        */
diff --git a/qemu-img.c b/qemu-img.c
index 28b27b738a..d8ca8a73e2 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -223,6 +223,32 @@ static bool qemu_img_object_print_help(const char *type, QemuOpts *opts)
     return true;
 }
 
+/*
+ * Is @optarg safe for accumulate_options()?
+ * It is when multiple of them can be joined together separated by ','.
+ * To make that work, @optarg must not start with ',' (or else a
+ * separating ',' preceding it gets escaped), and it must not end with
+ * an odd number of ',' (or else a separating ',' following it gets
+ * escaped).
+ */
+static bool is_valid_option_list(const char *optarg)
+{
+    size_t len = strlen(optarg);
+    size_t i;
+
+    if (optarg[0] == ',') {
+        return false;
+    }
+
+    for (i = len; i > 0 && optarg[i - 1] == ','; i--) {
+    }
+    if ((len - i) % 2) {
+        return false;
+    }
+
+    return true;
+}
+
 static int accumulate_options(char **options, char *optarg)
 {
     char *new_options;
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 279f1b3fb3..6f80807738 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -165,28 +165,6 @@ void parse_option_size(const char *name, const char *value,
     *ret = size;
 }
 
-bool is_valid_option_list(const char *p)
-{
-    char *value = NULL;
-    bool result = false;
-
-    while (*p) {
-        p = get_opt_value(p, &value);
-        if ((*p && !*++p) ||
-            (!*value || *value == ',')) {
-            goto out;
-        }
-
-        g_free(value);
-        value = NULL;
-    }
-
-    result = true;
-out:
-    g_free(value);
-    return result;
-}
-
 static const char *opt_type_to_string(enum QemuOptType type)
 {
     switch (type) {
-- 
2.21.1



  parent reply	other threads:[~2020-04-09 15:34 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-09 15:30 [PATCH for-5.1 0/8] qemu-option: Fix corner cases and clean up Markus Armbruster
2020-04-09 15:30 ` [PATCH for-5.1 1/8] tests-qemu-opts: Cover has_help_option(), qemu_opt_has_help_opt() Markus Armbruster
2020-04-09 17:50   ` Eric Blake
2020-04-14  9:10     ` Markus Armbruster
2020-04-14 13:13       ` Kevin Wolf
2020-04-14 13:36         ` Markus Armbruster
2020-04-14 14:29           ` Kevin Wolf
2020-04-14 20:13             ` Markus Armbruster
2020-04-15 10:00               ` Kevin Wolf
2020-04-15 14:45                 ` Markus Armbruster
2020-04-09 15:30 ` [PATCH for-5.1 2/8] qemu-options: Factor out get_opt_name_value() helper Markus Armbruster
2020-04-09 18:01   ` Eric Blake
2020-04-14  9:42     ` Markus Armbruster
2020-04-14 14:05       ` Kevin Wolf
2020-04-15  7:03         ` Markus Armbruster
2020-04-14 14:05   ` Kevin Wolf
2020-04-09 15:30 ` [PATCH for-5.1 3/8] qemu-option: Fix sloppy recognition of "id=..." after ", , " Markus Armbruster
2020-04-09 18:05   ` Eric Blake
2020-04-14 14:44   ` [PATCH for-5.1 3/8] qemu-option: Fix sloppy recognition of "id=..." after ",," Kevin Wolf
2020-04-09 15:30 ` [PATCH for-5.1 4/8] qemu-option: Avoid has_help_option() in qemu_opts_parse_noisily() Markus Armbruster
2020-04-09 18:07   ` Eric Blake
2020-04-14 10:04     ` Markus Armbruster
2020-04-09 15:30 ` [PATCH for-5.1 5/8] qemu-option: Fix has_help_option()'s sloppy parsing Markus Armbruster
2020-04-09 18:10   ` Eric Blake
2020-04-14 10:16     ` Markus Armbruster
2020-04-14 14:57       ` Kevin Wolf
2020-04-15  7:48         ` Markus Armbruster
2020-04-09 15:30 ` [PATCH for-5.1 6/8] test-qemu-opts: Simplify test_has_help_option() after bug fix Markus Armbruster
2020-04-09 18:13   ` Eric Blake
2020-04-14 14:58   ` Kevin Wolf
2020-04-09 15:30 ` [PATCH for-5.1 7/8] qemu-img: Factor out accumulate_options() helper Markus Armbruster
2020-04-09 18:15   ` Eric Blake
2020-04-14 15:00   ` Kevin Wolf
2020-04-09 15:30 ` Markus Armbruster [this message]
2020-04-09 19:45   ` [PATCH for-5.1 8/8] qemu-option: Move is_valid_option_list() to qemu-img.c and rewrite Eric Blake
2020-04-14  8:47     ` Markus Armbruster
2020-04-14 14:34       ` Markus Armbruster
2020-04-14 15:10         ` Kevin Wolf
2020-04-14 20:14           ` Markus Armbruster
2020-04-09 17:09 ` [PATCH for-5.1 0/8] qemu-option: Fix corner cases and clean up no-reply
2020-04-09 17:44   ` Eric Blake
2020-04-14  8:52     ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200409153041.17576-9-armbru@redhat.com \
    --to=armbru@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).