Patchwork [07/19] Never overwrite a QemuOpt

login
register
mail settings
Submitter Mark McLoughlin
Date Sept. 10, 2009, 3:18 p.m.
Message ID <1252595941-15196-8-git-send-email-markmc@redhat.com>
Download mbox | patch
Permalink /patch/33323/
State Superseded
Headers show

Comments

Mark McLoughlin - Sept. 10, 2009, 3:18 p.m.
Rather than overwriting a QemuOpt, just add a new one to the tail and
always do a reverse search for parameters to preserve the same
behaviour. We use this order so that foreach() iterates over the opts
in their original order.

This will allow us handle options where multiple values for the same
parameter is allowed - e.g. -net user,hostfwd=

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 qemu-option.c |   51 +++++++++++++++++++++++----------------------------
 1 files changed, 23 insertions(+), 28 deletions(-)
Gerd Hoffmann - Sept. 11, 2009, 7:51 a.m.
On 09/10/09 17:18, Mark McLoughlin wrote:
> Rather than overwriting a QemuOpt, just add a new one to the tail and
> always do a reverse search for parameters to preserve the same
> behaviour. We use this order so that foreach() iterates over the opts
> in their original order.

I think we should simply add a flag to QemuOptsDesc saying 'this can 
have multiple instances' instead of doing these reverse search tricks to 
maintain backward compatibility.

cheers,
   Gerd

Patch

diff --git a/qemu-option.c b/qemu-option.c
index 0f517d1..376efc1 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -483,7 +483,7 @@  struct QemuOpt {
 struct QemuOpts {
     const char *id;
     QemuOptsList *list;
-    TAILQ_HEAD(, QemuOpt) head;
+    TAILQ_HEAD(QemuOptHead, QemuOpt) head;
     TAILQ_ENTRY(QemuOpts) next;
 };
 
@@ -491,7 +491,7 @@  static QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name)
 {
     QemuOpt *opt;
 
-    TAILQ_FOREACH(opt, &opts->head, next) {
+    TAILQ_FOREACH_REVERSE(opt, &opts->head, QemuOptHead, next) {
         if (strcmp(opt->name, name) != 0)
             continue;
         return opt;
@@ -565,36 +565,31 @@  static void qemu_opt_del(QemuOpt *opt)
 int qemu_opt_set(QemuOpts *opts, const char *name, const char *value)
 {
     QemuOpt *opt;
+    QemuOptDesc *desc = opts->list->desc;
+    int i;
 
-    opt = qemu_opt_find(opts, name);
-    if (!opt) {
-        QemuOptDesc *desc = opts->list->desc;
-        int i;
-
-        for (i = 0; desc[i].name != NULL; i++) {
-            if (strcmp(desc[i].name, name) == 0) {
-                break;
-            }
-        }
-        if (desc[i].name == NULL) {
-            if (i == 0) {
-                /* empty list -> allow any */;
-            } else {
-                fprintf(stderr, "option \"%s\" is not valid for %s\n",
-                        name, opts->list->name);
-                return -1;
-            }
+    for (i = 0; desc[i].name != NULL; i++) {
+        if (strcmp(desc[i].name, name) == 0) {
+            break;
         }
-        opt = qemu_mallocz(sizeof(*opt));
-        opt->name = qemu_strdup(name);
-        opt->opts = opts;
-        TAILQ_INSERT_TAIL(&opts->head, opt, next);
-        if (desc[i].name != NULL) {
-            opt->desc = desc+i;
+    }
+    if (desc[i].name == NULL) {
+        if (i == 0) {
+            /* empty list -> allow any */;
+        } else {
+            fprintf(stderr, "option \"%s\" is not valid for %s\n",
+                    name, opts->list->name);
+            return -1;
         }
     }
-    qemu_free((/* !const */ char*)opt->str);
-    opt->str = NULL;
+
+    opt = qemu_mallocz(sizeof(*opt));
+    opt->name = qemu_strdup(name);
+    opt->opts = opts;
+    TAILQ_INSERT_TAIL(&opts->head, opt, next);
+    if (desc[i].name != NULL) {
+        opt->desc = desc+i;
+    }
     if (value) {
         opt->str = qemu_strdup(value);
     }