[03/11] keyval: introduce keyval_parse_into

Commit Message

Paolo Bonzini June 10, 2021, 1:35 p.m. UTC

Allow parsing multiple keyval sequences into the same dictionary.
This will be used to simplify the parsing of the -M command line
option, which is currently a .merge_lists = true QemuOpts group.

Signed-off-by: Paolo Bonzini <[e-mail]>
---
 include/qemu/option.h |  2 ++
 util/keyval.c         | 43 +++++++++++++++++++++++++++++++++++--------
 2 files changed, 37 insertions(+), 8 deletions(-)

Comments

Daniel P. Berrangé June 10, 2021, 1:53 p.m. UTC | #1

On Thu, Jun 10, 2021 at 03:35:30PM +0200, Paolo Bonzini wrote:
> Allow parsing multiple keyval sequences into the same dictionary.
> This will be used to simplify the parsing of the -M command line
> option, which is currently a .merge_lists = true QemuOpts group.
> 
> Signed-off-by: Paolo Bonzini <[e-mail]>
> ---
>  include/qemu/option.h |  2 ++
>  util/keyval.c         | 43 +++++++++++++++++++++++++++++++++++--------
>  2 files changed, 37 insertions(+), 8 deletions(-)

Reviewed-by: Daniel P. Berrangé <[e-mail]>


Regards,
Daniel

Markus Armbruster June 16, 2021, 2:09 p.m. UTC | #2

Paolo Bonzini <[e-mail]> writes:

> Allow parsing multiple keyval sequences into the same dictionary.
> This will be used to simplify the parsing of the -M command line
> option, which is currently a .merge_lists = true QemuOpts group.
>
> Signed-off-by: Paolo Bonzini <[e-mail]>

Straightforward.  Thanks for adjusting the contract for
keyval_parse_into().

Reviewed-by: Markus Armbruster <[e-mail]>

It's too hot & humid for me to figure out why you need both
keyval_merge() and keyval_parse_into().

Paolo Bonzini June 16, 2021, 6:05 p.m. UTC | #3

Il mer 16 giu 2021, 16:09 Markus Armbruster <[e-mail]> ha scritto:

> Paolo Bonzini <[e-mail]> writes:
>
> > Allow parsing multiple keyval sequences into the same dictionary.
> > This will be used to simplify the parsing of the -M command line
> > option, which is currently a .merge_lists = true QemuOpts group.
> >
> > Signed-off-by: Paolo Bonzini <[e-mail]>
>
> Straightforward.  Thanks for adjusting the contract for
> keyval_parse_into().
>
> Reviewed-by: Markus Armbruster <[e-mail]>
>
> It's too hot & humid for me to figure out why you need both
> keyval_merge() and keyval_parse_into().
>

I don't need both, but I do need keyval_merge() which is the complex one;
and keyval_parse_into() is straightforward enough to be worth the simpler
code in vl.c.

Paolo


>

Patch

diff --git a/include/qemu/option.h b/include/qemu/option.h
index d89c66145a..fffb03d848 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -147,6 +147,8 @@  void qemu_opts_print_help(QemuOptsList *list, bool print_caption);
 void qemu_opts_free(QemuOptsList *list);
 QemuOptsList *qemu_opts_append(QemuOptsList *dst, QemuOptsList *list);
 
+QDict *keyval_parse_into(QDict *qdict, const char *params, const char *implied_key,
+                         bool *p_help, Error **errp);
 QDict *keyval_parse(const char *params, const char *implied_key,
                     bool *help, Error **errp);
 void keyval_merge(QDict *old, const QDict *new, Error **errp);
diff --git a/util/keyval.c b/util/keyval.c
index 0797f36e1d..1ffd6e1204 100644
--- a/util/keyval.c
+++ b/util/keyval.c
@@ -478,13 +478,14 @@  static QObject *keyval_listify(QDict *cur, GSList *key_of_cur, Error **errp)
  * If @p_help is not NULL, store whether help is requested there.
  * If @p_help is NULL and help is requested, fail.
  *
- * On success, return a dictionary of the parsed keys and values.
- * On failure, store an error through @errp and return NULL.
+ * On success, return @dict, now filled with the parsed keys and values.
+ *
+ * On failure, store an error through @errp and return NULL.  Any keys
+ * and values parsed so far will be in @dict nevertheless.
  */
-QDict *keyval_parse(const char *params, const char *implied_key,
-                    bool *p_help, Error **errp)
+QDict *keyval_parse_into(QDict *qdict, const char *params, const char *implied_key,
+                         bool *p_help, Error **errp)
 {
-    QDict *qdict = qdict_new();
     QObject *listified;
     const char *s;
     bool help = false;
@@ -493,7 +494,6 @@  QDict *keyval_parse(const char *params, const char *implied_key,
     while (*s) {
         s = keyval_parse_one(qdict, s, implied_key, &help, errp);
         if (!s) {
-            qobject_unref(qdict);
             return NULL;
         }
         implied_key = NULL;
@@ -503,15 +503,42 @@  QDict *keyval_parse(const char *params, const char *implied_key,
         *p_help = help;
     } else if (help) {
         error_setg(errp, "Help is not available for this option");
-        qobject_unref(qdict);
         return NULL;
     }
 
     listified = keyval_listify(qdict, NULL, errp);
     if (!listified) {
-        qobject_unref(qdict);
         return NULL;
     }
     assert(listified == QOBJECT(qdict));
     return qdict;
 }
+
+/*
+ * Parse @params in QEMU's traditional KEY=VALUE,... syntax.
+ *
+ * If @implied_key, the first KEY= can be omitted.  @implied_key is
+ * implied then, and VALUE can't be empty or contain ',' or '='.
+ *
+ * A parameter "help" or "?" without a value isn't added to the
+ * resulting dictionary, but instead is interpreted as help request.
+ * All other options are parsed and returned normally so that context
+ * specific help can be printed.
+ *
+ * If @p_help is not NULL, store whether help is requested there.
+ * If @p_help is NULL and help is requested, fail.
+ *
+ * On success, return a dictionary of the parsed keys and values.
+ * On failure, store an error through @errp and return NULL.
+ */
+QDict *keyval_parse(const char *params, const char *implied_key,
+                    bool *p_help, Error **errp)
+{
+    QDict *qdict = qdict_new();
+    QDict *ret = keyval_parse_into(qdict, params, implied_key, p_help, errp);
+
+    if (!ret) {
+        qobject_unref(qdict);
+    }
+    return ret;
+}