[Qemu-devel] [PATCH 0/9] qdev deconstruction, command-line episode

[Qemu-devel] [PATCH 0/9] qdev deconstruction, command-line episode

[Paolo Bonzini]

This patch substitutes more qdev code with shared infrastructure.  The
code is now ripe enough that we can attack command-line parsing.

Parsing to a string is replaced with a StringInputVisitor (there is also
a StringOutputVisitor, but it's not used yet).  This lets us remove most
integer parsing/printing code; command-line parsing accesses the integer
properties with object_property_parse rather than object_property_set_int,
and that's enough in almost all cases.

Similar "type-casting visitors" could be created also for integer and
boolean and could replace the use of QObject in qom/object.c.

In addition, the limited polymorphism allowed by visitors is exploited
for PCI devfn properties.  These will always read as an integer (the
8-bit DDDDDFFF), but they can be set both as strings and integers.

While there are still a few legacy getters for "info qtree", the only
remaining legacy setters are for hex properties.  But even here we can
lay down the foundations for future simplification.  In general, they
are rarely used and their printed form is more interesting than the
parsing.  For example you'd usually set isa-serial.index instead of
isa-serial.iobase.  Plus (luckily) our main client, libvirt, only cares
about few of these, and always sets them with a 0x prefix.  So the series
stops accepting bare hexadecimal numbers, preparing for making legacy
properties read-only in 1.3 or so.

Patches 1 to 4 refactor some QAPI code and introduce the string visitors.
Patches 5 uses them in new generic property accessors.  Patches 6 to 9
put the shiny new accessors to use in qdev.

Paolo Bonzini (9):
  qapi: allow sharing enum implementation across visitors
  qapi: drop qmp_input_end_optional
  qapi: add string-based visitors
  qapi: add tests for string-based visitors
  qom: add generic string parsing/printing
  qdev: accept both strings and integers for PCI addresses
  qdev: accept hex properties only if prefixed by 0x
  qdev: use built-in QOM string parser
  qdev: drop unnecessary parse/print methods

 .gitignore                   |    2 +
 Makefile.objs                |    5 +-
 hw/qdev-properties.c         |  186 +++++++++---------------------------------
 include/qemu/object.h        |   24 ++++++
 qapi/qapi-visit-core.c       |   52 ++++++++++++
 qapi/qapi-visit-impl.h       |   23 +++++
 qapi/qmp-input-visitor.c     |   39 +--------
 qapi/qmp-output-visitor.c    |   22 +-----
 qapi/string-input-visitor.c  |  136 ++++++++++++++++++++++++++++++
 qapi/string-input-visitor.h  |   25 ++++++
 qapi/string-output-visitor.c |   89 ++++++++++++++++++++
 qapi/string-output-visitor.h |   26 ++++++
 qom/object.c                 |   24 ++++++
 test-string-input-visitor.c  |  160 +++++++++++++++++++++++++++++++++++
 test-string-output-visitor.c |  188 ++++++++++++++++++++++++++++++++++++++++++
 tests/Makefile               |   12 ++-
 16 files changed, 805 insertions(+), 208 deletions(-)
 create mode 100644 qapi/qapi-visit-impl.h
 create mode 100644 qapi/string-input-visitor.c
 create mode 100644 qapi/string-input-visitor.h
 create mode 100644 qapi/string-output-visitor.c
 create mode 100644 qapi/string-output-visitor.h
 create mode 100644 test-string-input-visitor.c
 create mode 100644 test-string-output-visitor.c

-- 
1.7.7.6

[Qemu-devel] [PATCH 1/9] qapi: allow sharing enum implementation across visitors

[Paolo Bonzini]

Most visitors will use the same code for enum parsing.  Move it to
the core.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qapi/qapi-visit-core.c    |   51 +++++++++++++++++++++++++++++++++++++++++++++
 qapi/qapi-visit-impl.h    |   23 ++++++++++++++++++++
 qapi/qmp-input-visitor.c  |   34 +----------------------------
 qapi/qmp-output-visitor.c |   22 +-----------------
 4 files changed, 78 insertions(+), 52 deletions(-)
 create mode 100644 qapi/qapi-visit-impl.h

diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index ddef3ed..a4e088c 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -12,6 +12,7 @@
  */
 
 #include "qapi/qapi-visit-core.h"
+#include "qapi/qapi-visit-impl.h"
 
 void visit_start_handle(Visitor *v, void **obj, const char *kind,
                         const char *name, Error **errp)
@@ -116,3 +117,53 @@ void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp)
         v->type_number(v, obj, name, errp);
     }
 }
+
+void output_type_enum(Visitor *v, int *obj, const char *strings[],
+                      const char *kind, const char *name,
+                      Error **errp)
+{
+    int i = 0;
+    int value = *obj;
+    char *enum_str;
+
+    assert(strings);
+    while (strings[i++] != NULL);
+    if (value < 0 || value >= i - 1) {
+        error_set(errp, QERR_INVALID_PARAMETER, name ? name : "null");
+        return;
+    }
+
+    enum_str = (char *)strings[value];
+    visit_type_str(v, &enum_str, name, errp);
+}
+
+void input_type_enum(Visitor *v, int *obj, const char *strings[],
+                     const char *kind, const char *name,
+                     Error **errp)
+{
+    int64_t value = 0;
+    char *enum_str;
+
+    assert(strings);
+
+    visit_type_str(v, &enum_str, name, errp);
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    while (strings[value] != NULL) {
+        if (strcmp(strings[value], enum_str) == 0) {
+            break;
+        }
+        value++;
+    }
+
+    if (strings[value] == NULL) {
+        error_set(errp, QERR_INVALID_PARAMETER, name ? name : "null");
+        g_free(enum_str);
+        return;
+    }
+
+    g_free(enum_str);
+    *obj = value;
+}
diff --git a/qapi/qapi-visit-impl.h b/qapi/qapi-visit-impl.h
new file mode 100644
index 0000000..0f3a189
--- /dev/null
+++ b/qapi/qapi-visit-impl.h
@@ -0,0 +1,23 @@
+/*
+ * Core Definitions for QAPI Visitor implementations
+ *
+ * Copyright (C) 2012 Red Hat, Inc.
+ *
+ * Author: Paolo Bonizni <pbonzini@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+#ifndef QAPI_VISITOR_IMPL_H
+#define QAPI_VISITOR_IMPL_H
+
+#include "qapi/qapi-types-core.h"
+#include "qapi/qapi-visit-core.h"
+
+void input_type_enum(Visitor *v, int *obj, const char *strings[],
+                     const char *kind, const char *name, Error **errp);
+void output_type_enum(Visitor *v, int *obj, const char *strings[],
+                      const char *kind, const char *name, Error **errp);
+
+#endif
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index c78022b..012cd0a 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -12,6 +12,7 @@
  */
 
 #include "qmp-input-visitor.h"
+#include "qapi/qapi-visit-impl.h"
 #include "qemu-queue.h"
 #include "qemu-common.h"
 #include "qemu-objects.h"
@@ -217,37 +218,6 @@ static void qmp_input_type_number(Visitor *v, double *obj, const char *name,
     *obj = qfloat_get_double(qobject_to_qfloat(qobj));
 }
 
-static void qmp_input_type_enum(Visitor *v, int *obj, const char *strings[],
-                                const char *kind, const char *name,
-                                Error **errp)
-{
-    int64_t value = 0;
-    char *enum_str;
-
-    assert(strings);
-
-    qmp_input_type_str(v, &enum_str, name, errp);
-    if (error_is_set(errp)) {
-        return;
-    }
-
-    while (strings[value] != NULL) {
-        if (strcmp(strings[value], enum_str) == 0) {
-            break;
-        }
-        value++;
-    }
-
-    if (strings[value] == NULL) {
-        error_set(errp, QERR_INVALID_PARAMETER, name ? name : "null");
-        g_free(enum_str);
-        return;
-    }
-
-    g_free(enum_str);
-    *obj = value;
-}
-
 static void qmp_input_start_optional(Visitor *v, bool *present,
                                      const char *name, Error **errp)
 {
@@ -288,7 +258,7 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj)
     v->visitor.start_list = qmp_input_start_list;
     v->visitor.next_list = qmp_input_next_list;
     v->visitor.end_list = qmp_input_end_list;
-    v->visitor.type_enum = qmp_input_type_enum;
+    v->visitor.type_enum = input_type_enum;
     v->visitor.type_int = qmp_input_type_int;
     v->visitor.type_bool = qmp_input_type_bool;
     v->visitor.type_str = qmp_input_type_str;
diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index f76d015..e0697b0 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -12,6 +12,7 @@
  */
 
 #include "qmp-output-visitor.h"
+#include "qapi/qapi-visit-impl.h"
 #include "qemu-queue.h"
 #include "qemu-common.h"
 #include "qemu-objects.h"
@@ -180,25 +181,6 @@ static void qmp_output_type_number(Visitor *v, double *obj, const char *name,
     qmp_output_add(qov, name, qfloat_from_double(*obj));
 }
 
-static void qmp_output_type_enum(Visitor *v, int *obj, const char *strings[],
-                                 const char *kind, const char *name,
-                                 Error **errp)
-{
-    int i = 0;
-    int value = *obj;
-    char *enum_str;
-
-    assert(strings);
-    while (strings[i++] != NULL);
-    if (value < 0 || value >= i - 1) {
-        error_set(errp, QERR_INVALID_PARAMETER, name ? name : "null");
-        return;
-    }
-
-    enum_str = (char *)strings[value];
-    qmp_output_type_str(v, &enum_str, name, errp);
-}
-
 QObject *qmp_output_get_qobject(QmpOutputVisitor *qov)
 {
     QObject *obj = qmp_output_first(qov);
@@ -239,7 +221,7 @@ QmpOutputVisitor *qmp_output_visitor_new(void)
     v->visitor.start_list = qmp_output_start_list;
     v->visitor.next_list = qmp_output_next_list;
     v->visitor.end_list = qmp_output_end_list;
-    v->visitor.type_enum = qmp_output_type_enum;
+    v->visitor.type_enum = output_type_enum;
     v->visitor.type_int = qmp_output_type_int;
     v->visitor.type_bool = qmp_output_type_bool;
     v->visitor.type_str = qmp_output_type_str;
-- 
1.7.7.6

[Qemu-devel] [PATCH 2/9] qapi: drop qmp_input_end_optional

[Paolo Bonzini]

This method is optional, do not implement it if it is empty.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qapi/qmp-input-visitor.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 012cd0a..e6b6152 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -232,10 +232,6 @@ static void qmp_input_start_optional(Visitor *v, bool *present,
     *present = true;
 }
 
-static void qmp_input_end_optional(Visitor *v, Error **errp)
-{
-}
-
 Visitor *qmp_input_get_visitor(QmpInputVisitor *v)
 {
     return &v->visitor;
@@ -264,7 +260,6 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj)
     v->visitor.type_str = qmp_input_type_str;
     v->visitor.type_number = qmp_input_type_number;
     v->visitor.start_optional = qmp_input_start_optional;
-    v->visitor.end_optional = qmp_input_end_optional;
 
     v->obj = obj;
     qobject_incref(v->obj);
-- 
1.7.7.6

[Qemu-devel] [PATCH 3/9] qapi: add string-based visitors

[Paolo Bonzini]

String based visitors provide a consistent interface for parsing
strings to C values, as well as consuming C values as strings.
They will be used to parse command-line options.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile.objs                |    5 +-
 qapi/string-input-visitor.c  |  136 ++++++++++++++++++++++++++++++++++++++++++
 qapi/string-input-visitor.h  |   25 ++++++++
 qapi/string-output-visitor.c |   89 ++++++++++++++++++++++++++++
 qapi/string-output-visitor.h |   26 ++++++++
 6 files changed, 280 insertions(+), 2 deletions(-)
 create mode 100644 qapi/string-input-visitor.c
 create mode 100644 qapi/string-input-visitor.h
 create mode 100644 qapi/string-output-visitor.c
 create mode 100644 qapi/string-output-visitor.h

diff --git a/Makefile.objs b/Makefile.objs
index ec35320..8f9e1fb 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -414,8 +414,9 @@ libcacard-y = cac.o event.o vcard.o vreader.o vcard_emul_nss.o vcard_emul_type.o
 ######################################################################
 # qapi
 
-qapi-nested-y = qapi-visit-core.o qmp-input-visitor.o qmp-output-visitor.o qapi-dealloc-visitor.o
-qapi-nested-y += qmp-registry.o qmp-dispatch.o
+qapi-nested-y = qapi-visit-core.o qapi-dealloc-visitor.o qmp-input-visitor.o
+qapi-nested-y += qmp-output-visitor.o qmp-registry.o qmp-dispatch.o
+qapi-nested-y += string-input-visitor.o string-output-visitor.o
 qapi-obj-y = $(addprefix qapi/, $(qapi-nested-y))
 
 common-obj-y += qmp-marshal.o qapi-visit.o qapi-types.o $(qapi-obj-y)
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
new file mode 100644
index 0000000..9ca4087
--- /dev/null
+++ b/qapi/string-input-visitor.c
@@ -0,0 +1,136 @@
+/*
+ * String parsing visitor
+ *
+ * Copyright Red Hat, Inc. 2012
+ *
+ * Author: Paolo Bonzini <pbonzini@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "qemu-common.h"
+#include "string-input-visitor.h"
+#include "qapi/qapi-visit-impl.h"
+#include "qerror.h"
+
+struct StringInputVisitor
+{
+    Visitor visitor;
+    const char *string;
+};
+
+static void parse_type_int(Visitor *v, int64_t *obj, const char *name,
+                           Error **errp)
+{
+    StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
+    char *endp = (char *) siv->string;
+    long long val;
+
+    errno = 0;
+    if (siv->string) {
+        val = strtoll(siv->string, &endp, 0);
+    }
+    if (!siv->string || errno || endp == siv->string || *endp) {
+        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
+                  "integer");
+        return;
+    }
+
+    *obj = val;
+}
+
+static void parse_type_bool(Visitor *v, bool *obj, const char *name,
+                            Error **errp)
+{
+    StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
+
+    if (siv->string) {
+        if (strcasecmp(siv->string, "on") || strcasecmp(siv->string, "yes") ||
+            strcasecmp(siv->string, "true")) {
+            *obj = true;
+            return;
+        }
+        if (strcasecmp(siv->string, "off") || strcasecmp(siv->string, "no") ||
+            strcasecmp(siv->string, "false")) {
+            *obj = false;
+            return;
+        }
+    }
+
+    error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
+              "boolean");
+}
+
+static void parse_type_str(Visitor *v, char **obj, const char *name,
+                           Error **errp)
+{
+    StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
+    if (siv->string) {
+        *obj = g_strdup(siv->string);
+    } else {
+        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
+                  "string");
+    }
+}
+
+static void parse_type_number(Visitor *v, double *obj, const char *name,
+                              Error **errp)
+{
+    StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
+    char *endp = (char *) siv->string;
+    double val;
+
+    errno = 0;
+    if (siv->string) {
+        val = strtod(siv->string, &endp);
+    }
+    if (!siv->string || errno || endp == siv->string || *endp) {
+        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
+                  "number");
+        return;
+    }
+
+    *obj = val;
+}
+
+static void parse_start_optional(Visitor *v, bool *present,
+                                 const char *name, Error **errp)
+{
+    StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
+
+    if (!siv->string) {
+        *present = false;
+        return;
+    }
+
+    *present = true;
+}
+
+Visitor *string_input_get_visitor(StringInputVisitor *v)
+{
+    return &v->visitor;
+}
+
+void string_input_visitor_cleanup(StringInputVisitor *v)
+{
+    g_free(v);
+}
+
+StringInputVisitor *string_input_visitor_new(const char *str)
+{
+    StringInputVisitor *v;
+
+    v = g_malloc0(sizeof(*v));
+
+    v->visitor.type_enum = input_type_enum;
+    v->visitor.type_int = parse_type_int;
+    v->visitor.type_bool = parse_type_bool;
+    v->visitor.type_str = parse_type_str;
+    v->visitor.type_number = parse_type_number;
+    v->visitor.start_optional = parse_start_optional;
+
+    v->string = str;
+    return v;
+}
diff --git a/qapi/string-input-visitor.h b/qapi/string-input-visitor.h
new file mode 100644
index 0000000..d269d42
--- /dev/null
+++ b/qapi/string-input-visitor.h
@@ -0,0 +1,25 @@
+/*
+ * String parsing Visitor
+ *
+ * Copyright Red Hat, Inc. 2012
+ *
+ * Author: Paolo Bonzini <pbonzini@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef STRING_INPUT_VISITOR_H
+#define STRING_INPUT_VISITOR_H
+
+#include "qapi-visit-core.h"
+
+typedef struct StringInputVisitor StringInputVisitor;
+
+StringInputVisitor *string_input_visitor_new(const char *str);
+void string_input_visitor_cleanup(StringInputVisitor *v);
+
+Visitor *string_input_get_visitor(StringInputVisitor *v);
+
+#endif
diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
new file mode 100644
index 0000000..6fa72f3
--- /dev/null
+++ b/qapi/string-output-visitor.c
@@ -0,0 +1,89 @@
+/*
+ * String printing Visitor
+ *
+ * Copyright Red Hat, Inc. 2012
+ *
+ * Author: Paolo Bonzini <pbonzini@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "qemu-common.h"
+#include "string-output-visitor.h"
+#include "qapi/qapi-visit-impl.h"
+#include "qerror.h"
+
+struct StringOutputVisitor
+{
+    Visitor visitor;
+    char *string;
+};
+
+static void string_output_set(StringOutputVisitor *sov, char *string)
+{
+    g_free(sov->string);
+    sov->string = string;
+}
+
+static void print_type_int(Visitor *v, int64_t *obj, const char *name,
+                           Error **errp)
+{
+    StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
+    string_output_set(sov, g_strdup_printf("%lld", (long long) *obj));
+}
+
+static void print_type_bool(Visitor *v, bool *obj, const char *name,
+                            Error **errp)
+{
+    StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
+    string_output_set(sov, g_strdup(*obj ? "true" : "false"));
+}
+
+static void print_type_str(Visitor *v, char **obj, const char *name,
+                           Error **errp)
+{
+    StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
+    string_output_set(sov, g_strdup(*obj ? *obj : ""));
+}
+
+static void print_type_number(Visitor *v, double *obj, const char *name,
+                              Error **errp)
+{
+    StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
+    string_output_set(sov, g_strdup_printf("%g", *obj));
+}
+
+char *string_output_get_string(StringOutputVisitor *sov)
+{
+    char *string = sov->string;
+    sov->string = NULL;
+    return string;
+}
+
+Visitor *string_output_get_visitor(StringOutputVisitor *sov)
+{
+    return &sov->visitor;
+}
+
+void string_output_visitor_cleanup(StringOutputVisitor *sov)
+{
+    g_free(sov->string);
+    g_free(sov);
+}
+
+StringOutputVisitor *string_output_visitor_new(void)
+{
+    StringOutputVisitor *v;
+
+    v = g_malloc0(sizeof(*v));
+
+    v->visitor.type_enum = output_type_enum;
+    v->visitor.type_int = print_type_int;
+    v->visitor.type_bool = print_type_bool;
+    v->visitor.type_str = print_type_str;
+    v->visitor.type_number = print_type_number;
+
+    return v;
+}
diff --git a/qapi/string-output-visitor.h b/qapi/string-output-visitor.h
new file mode 100644
index 0000000..05b3b29
--- /dev/null
+++ b/qapi/string-output-visitor.h
@@ -0,0 +1,26 @@
+/*
+ * String printing Visitor
+ *
+ * Copyright Red Hat, Inc. 2012
+ *
+ * Author: Paolo Bonzini <pbonzini@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef STRING_OUTPUT_VISITOR_H
+#define STRING_OUTPUT_VISITOR_H
+
+#include "qapi-visit-core.h"
+
+typedef struct StringOutputVisitor StringOutputVisitor;
+
+StringOutputVisitor *string_output_visitor_new(void);
+void string_output_visitor_cleanup(StringOutputVisitor *v);
+
+char *string_output_get_string(StringOutputVisitor *v);
+Visitor *string_output_get_visitor(StringOutputVisitor *v);
+
+#endif
-- 
1.7.7.6

[Qemu-devel] [PATCH 4/9] qapi: add tests for string-based visitors

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 .gitignore                   |    2 +
 test-string-input-visitor.c  |  160 +++++++++++++++++++++++++++++++++++
 test-string-output-visitor.c |  188 ++++++++++++++++++++++++++++++++++++++++++
 tests/Makefile               |   12 ++-
 4 files changed, 360 insertions(+), 2 deletions(-)
 create mode 100644 test-string-input-visitor.c
 create mode 100644 test-string-output-visitor.c

diff --git a/.gitignore b/.gitignore
index f5aab2c..c72955a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -44,6 +44,8 @@ QMP/qmp-commands.txt
 test-coroutine
 test-qmp-input-visitor
 test-qmp-output-visitor
+test-string-input-visitor
+test-string-output-visitor
 fsdev/virtfs-proxy-helper.1
 fsdev/virtfs-proxy-helper.pod
 .gdbinit
diff --git a/test-string-input-visitor.c b/test-string-input-visitor.c
new file mode 100644
index 0000000..1be6b2f
--- /dev/null
+++ b/test-string-input-visitor.c
@@ -0,0 +1,160 @@
+/*
+ * String Input Visitor unit-tests.
+ *
+ * Copyright (C) 2012 Red Hat Inc.
+ *
+ * Authors:
+ *  Paolo Bonzini <pbonzini@redhat.com> (based on test-qmp-input-visitor)
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include <glib.h>
+#include <stdarg.h>
+
+#include "qapi/string-input-visitor.h"
+#include "test-qapi-types.h"
+#include "test-qapi-visit.h"
+#include "qemu-objects.h"
+
+typedef struct TestInputVisitorData {
+    StringInputVisitor *siv;
+} TestInputVisitorData;
+
+static void visitor_input_teardown(TestInputVisitorData *data,
+                                   const void *unused)
+{
+    if (data->siv) {
+        string_input_visitor_cleanup(data->siv);
+        data->siv = NULL;
+    }
+}
+
+/* This is provided instead of a test setup function so that the JSON
+   string used by the tests are kept in the test functions (and not
+   int main()) */
+static
+Visitor *visitor_input_test_init(TestInputVisitorData *data,
+                                 const char *string)
+{
+    Visitor *v;
+
+    data->siv = string_input_visitor_new(string);
+    g_assert(data->siv != NULL);
+
+    v = string_input_get_visitor(data->siv);
+    g_assert(v != NULL);
+
+    return v;
+}
+
+static void test_visitor_in_int(TestInputVisitorData *data,
+                                const void *unused)
+{
+    int64_t res = 0, value = -42;
+    Error *errp = NULL;
+    Visitor *v;
+
+    v = visitor_input_test_init(data, "-42");
+
+    visit_type_int(v, &res, NULL, &errp);
+    g_assert(!error_is_set(&errp));
+    g_assert_cmpint(res, ==, value);
+}
+
+static void test_visitor_in_bool(TestInputVisitorData *data,
+                                 const void *unused)
+{
+    Error *errp = NULL;
+    bool res = false;
+    Visitor *v;
+
+    v = visitor_input_test_init(data, "true");
+
+    visit_type_bool(v, &res, NULL, &errp);
+    g_assert(!error_is_set(&errp));
+    g_assert_cmpint(res, ==, true);
+}
+
+static void test_visitor_in_number(TestInputVisitorData *data,
+                                   const void *unused)
+{
+    double res = 0, value = 3.14;
+    Error *errp = NULL;
+    Visitor *v;
+
+    v = visitor_input_test_init(data, "3.14");
+
+    visit_type_number(v, &res, NULL, &errp);
+    g_assert(!error_is_set(&errp));
+    g_assert_cmpfloat(res, ==, value);
+}
+
+static void test_visitor_in_string(TestInputVisitorData *data,
+                                   const void *unused)
+{
+    char *res = NULL, *value = (char *) "Q E M U";
+    Error *errp = NULL;
+    Visitor *v;
+
+    v = visitor_input_test_init(data, value);
+
+    visit_type_str(v, &res, NULL, &errp);
+    g_assert(!error_is_set(&errp));
+    g_assert_cmpstr(res, ==, value);
+
+    g_free(res);
+}
+
+static void test_visitor_in_enum(TestInputVisitorData *data,
+                                 const void *unused)
+{
+    Error *errp = NULL;
+    Visitor *v;
+    EnumOne i;
+
+    for (i = 0; EnumOne_lookup[i]; i++) {
+        EnumOne res = -1;
+
+        v = visitor_input_test_init(data, EnumOne_lookup[i]);
+
+        visit_type_EnumOne(v, &res, NULL, &errp);
+        g_assert(!error_is_set(&errp));
+        g_assert_cmpint(i, ==, res);
+
+        visitor_input_teardown(data, NULL);
+    }
+
+    data->siv = NULL;
+}
+
+static void input_visitor_test_add(const char *testpath,
+                                   TestInputVisitorData *data,
+                                   void (*test_func)(TestInputVisitorData *data, const void *user_data))
+{
+    g_test_add(testpath, TestInputVisitorData, data, NULL, test_func,
+               visitor_input_teardown);
+}
+
+int main(int argc, char **argv)
+{
+    TestInputVisitorData in_visitor_data;
+
+    g_test_init(&argc, &argv, NULL);
+
+    input_visitor_test_add("/string-visitor/input/int",
+                           &in_visitor_data, test_visitor_in_int);
+    input_visitor_test_add("/string-visitor/input/bool",
+                           &in_visitor_data, test_visitor_in_bool);
+    input_visitor_test_add("/string-visitor/input/number",
+                           &in_visitor_data, test_visitor_in_number);
+    input_visitor_test_add("/string-visitor/input/string",
+                            &in_visitor_data, test_visitor_in_string);
+    input_visitor_test_add("/string-visitor/input/enum",
+                            &in_visitor_data, test_visitor_in_enum);
+
+    g_test_run();
+
+    return 0;
+}
diff --git a/test-string-output-visitor.c b/test-string-output-visitor.c
new file mode 100644
index 0000000..f56bed0
--- /dev/null
+++ b/test-string-output-visitor.c
@@ -0,0 +1,188 @@
+/*
+ * String Output Visitor unit-tests.
+ *
+ * Copyright (C) 2012 Red Hat Inc.
+ *
+ * Authors:
+ *  Paolo Bonzini <pbonzini@redhat.com> (based on test-qmp-output-visitor)
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include <glib.h>
+
+#include "qapi/string-output-visitor.h"
+#include "test-qapi-types.h"
+#include "test-qapi-visit.h"
+#include "qemu-objects.h"
+
+typedef struct TestOutputVisitorData {
+    StringOutputVisitor *sov;
+    Visitor *ov;
+} TestOutputVisitorData;
+
+static void visitor_output_setup(TestOutputVisitorData *data,
+                                 const void *unused)
+{
+    data->sov = string_output_visitor_new();
+    g_assert(data->sov != NULL);
+
+    data->ov = string_output_get_visitor(data->sov);
+    g_assert(data->ov != NULL);
+}
+
+static void visitor_output_teardown(TestOutputVisitorData *data,
+                                    const void *unused)
+{
+    string_output_visitor_cleanup(data->sov);
+    data->sov = NULL;
+    data->ov = NULL;
+}
+
+static void test_visitor_out_int(TestOutputVisitorData *data,
+                                 const void *unused)
+{
+    int64_t value = -42;
+    Error *errp = NULL;
+    char *str;
+
+    visit_type_int(data->ov, &value, NULL, &errp);
+    g_assert(error_is_set(&errp) == 0);
+
+    str = string_output_get_string(data->sov);
+    g_assert(str != NULL);
+    g_assert_cmpstr(str, ==, "-42");
+    g_free(str);
+}
+
+static void test_visitor_out_bool(TestOutputVisitorData *data,
+                                  const void *unused)
+{
+    Error *errp = NULL;
+    bool value = true;
+    char *str;
+
+    visit_type_bool(data->ov, &value, NULL, &errp);
+    g_assert(error_is_set(&errp) == 0);
+
+    str = string_output_get_string(data->sov);
+    g_assert(str != NULL);
+    g_assert_cmpstr(str, ==, "true");
+    g_free(str);
+}
+
+static void test_visitor_out_number(TestOutputVisitorData *data,
+                                    const void *unused)
+{
+    double value = 3.14;
+    Error *errp = NULL;
+    char *str;
+
+    visit_type_number(data->ov, &value, NULL, &errp);
+    g_assert(error_is_set(&errp) == 0);
+
+    str = string_output_get_string(data->sov);
+    g_assert(str != NULL);
+    g_assert_cmpstr(str, ==, "3.14");
+    g_free(str);
+}
+
+static void test_visitor_out_string(TestOutputVisitorData *data,
+                                    const void *unused)
+{
+    char *string = (char *) "Q E M U";
+    Error *errp = NULL;
+    char *str;
+
+    visit_type_str(data->ov, &string, NULL, &errp);
+    g_assert(error_is_set(&errp) == 0);
+
+    str = string_output_get_string(data->sov);
+    g_assert(str != NULL);
+    g_assert_cmpstr(str, ==, string);
+    g_free(str);
+}
+
+static void test_visitor_out_no_string(TestOutputVisitorData *data,
+                                       const void *unused)
+{
+    char *string = NULL;
+    Error *errp = NULL;
+    char *str;
+
+    /* A null string should return "" */
+    visit_type_str(data->ov, &string, NULL, &errp);
+    g_assert(error_is_set(&errp) == 0);
+
+    str = string_output_get_string(data->sov);
+    g_assert(str != NULL);
+    g_assert_cmpstr(str, ==, "");
+    g_free(str);
+}
+
+static void test_visitor_out_enum(TestOutputVisitorData *data,
+                                  const void *unused)
+{
+    Error *errp = NULL;
+    char *str;
+    EnumOne i;
+
+    for (i = 0; i < ENUM_ONE_MAX; i++) {
+        visit_type_EnumOne(data->ov, &i, "unused", &errp);
+        g_assert(!error_is_set(&errp));
+
+        str = string_output_get_string(data->sov);
+        g_assert(str != NULL);
+        g_assert_cmpstr(str, ==, EnumOne_lookup[i]);
+	g_free(str);
+    }
+}
+
+static void test_visitor_out_enum_errors(TestOutputVisitorData *data,
+                                         const void *unused)
+{
+    EnumOne i, bad_values[] = { ENUM_ONE_MAX, -1 };
+    Error *errp;
+
+    for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) {
+        errp = NULL;
+        visit_type_EnumOne(data->ov, &bad_values[i], "unused", &errp);
+        g_assert(error_is_set(&errp) == true);
+        error_free(errp);
+    }
+}
+
+static void output_visitor_test_add(const char *testpath,
+                                    TestOutputVisitorData *data,
+                                    void (*test_func)(TestOutputVisitorData *data, const void *user_data))
+{
+    g_test_add(testpath, TestOutputVisitorData, data, visitor_output_setup,
+               test_func, visitor_output_teardown);
+}
+
+int main(int argc, char **argv)
+{
+    TestOutputVisitorData out_visitor_data;
+
+    g_test_init(&argc, &argv, NULL);
+
+    output_visitor_test_add("/string-visitor/output/int",
+                            &out_visitor_data, test_visitor_out_int);
+    output_visitor_test_add("/string-visitor/output/bool",
+                            &out_visitor_data, test_visitor_out_bool);
+    output_visitor_test_add("/string-visitor/output/number",
+                            &out_visitor_data, test_visitor_out_number);
+    output_visitor_test_add("/string-visitor/output/string",
+                            &out_visitor_data, test_visitor_out_string);
+    output_visitor_test_add("/string-visitor/output/no-string",
+                            &out_visitor_data, test_visitor_out_no_string);
+    output_visitor_test_add("/string-visitor/output/enum",
+                            &out_visitor_data, test_visitor_out_enum);
+    output_visitor_test_add("/string-visitor/output/enum-errors",
+                            &out_visitor_data, test_visitor_out_enum_errors);
+
+    g_test_run();
+
+    return 0;
+}
diff --git a/tests/Makefile b/tests/Makefile
index 55e8eb0..74b29dc 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -1,6 +1,6 @@
 CHECKS = check-qdict check-qfloat check-qint check-qstring check-qlist
 CHECKS += check-qjson test-qmp-output-visitor test-qmp-input-visitor
-CHECKS += test-coroutine
+CHECKS += test-string-input-visitor test-string-output-visitor test-coroutine
 
 check-qint.o check-qstring.o check-qdict.o check-qlist.o check-qfloat.o check-qjson.o test-coroutine.o: $(GENERATED_HEADERS)
 
@@ -12,7 +12,9 @@ check-qfloat: check-qfloat.o qfloat.o $(tools-obj-y)
 check-qjson: check-qjson.o $(qobject-obj-y) $(tools-obj-y)
 test-coroutine: test-coroutine.o qemu-timer-common.o async.o $(coroutine-obj-y) $(tools-obj-y)
 
-test-qmp-input-visitor.o test-qmp-output-visitor.o test-qmp-commands.o qemu-ga$(EXESUF): QEMU_CFLAGS += -I $(qapi-dir)
+test-qmp-input-visitor.o test-qmp-output-visitor.o \
+test-string-input-visitor.o test-string-output-visitor.o \
+	test-qmp-commands.o qemu-ga$(EXESUF): QEMU_CFLAGS += -I $(qapi-dir)
 
 $(qapi-dir)/test-qapi-types.c $(qapi-dir)/test-qapi-types.h :\
 $(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py
@@ -25,6 +27,12 @@ $(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-commands.py
 	    $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -o "$(qapi-dir)" -p "test-" < $<, "  GEN   $@")
 
 
+test-string-output-visitor.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h) $(qapi-obj-y)
+test-string-output-visitor: test-string-output-visitor.o $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o
+
+test-string-input-visitor.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h) $(qapi-obj-y)
+test-string-input-visitor: test-string-input-visitor.o $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o
+
 test-qmp-output-visitor.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h) $(qapi-obj-y)
 test-qmp-output-visitor: test-qmp-output-visitor.o $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o
 
-- 
1.7.7.6

[Qemu-devel] [PATCH 5/9] qom: add generic string parsing/printing

[Paolo Bonzini]

Add generic property accessors that take a string and parse it
appropriately for the property type.  All the magic here is done
by the new string-based visitors.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qemu/object.h |   24 ++++++++++++++++++++++++
 qom/object.c          |   24 ++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/include/qemu/object.h b/include/qemu/object.h
index 7d50da9..af071eb 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -730,6 +730,30 @@ void object_property_set(Object *obj, struct Visitor *v, const char *name,
                          struct Error **errp);
 
 /**
+ * object_property_parse:
+ * @obj: the object
+ * @string: the string that will be used to parse the property value.
+ * @name: the name of the property
+ * @errp: returns an error if this function fails
+ *
+ * Parses a string and writes the result into a property of an object.
+ */
+void object_property_parse(Object *obj, const char *string,
+                           const char *name, struct Error **errp);
+
+/**
+ * object_property_set:
+ * @obj: the object
+ * @name: the name of the property
+ * @errp: returns an error if this function fails
+ *
+ * Returns a string representation of the value of the property.  The
+ * caller shall free the string.
+ */
+char *object_property_print(Object *obj, const char *name,
+                            struct Error **errp);
+
+/**
  * @object_property_get_type:
  * @obj: the object
  * @name: the name of the property
diff --git a/qom/object.c b/qom/object.c
index 5e5b261..97d898f 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -13,6 +13,8 @@
 #include "qemu/object.h"
 #include "qemu-common.h"
 #include "qapi/qapi-visit-core.h"
+#include "qapi/string-input-visitor.h"
+#include "qapi/string-output-visitor.h"
 
 /* TODO: replace QObject with a simpler visitor to avoid a dependency
  * of the QOM core on QObject?  */
@@ -782,6 +784,28 @@ int64_t object_property_get_int(Object *obj, const char *name,
     return retval;
 }
 
+void object_property_parse(Object *obj, const char *string,
+                           const char *name, Error **errp)
+{
+    StringInputVisitor *mi;
+    mi = string_input_visitor_new(string);
+    object_property_set(obj, string_input_get_visitor(mi), name, errp);
+
+    string_input_visitor_cleanup(mi);
+}
+
+char *object_property_print(Object *obj, const char *name,
+                            Error **errp)
+{
+    StringOutputVisitor *mo;
+    char *string;
+
+    mo = string_output_visitor_new();
+    object_property_get(obj, string_output_get_visitor(mo), name, NULL);
+    string = string_output_get_string(mo);
+    string_output_visitor_cleanup(mo);
+    return string;
+}
 const char *object_property_get_type(Object *obj, const char *name, Error **errp)
 {
     ObjectProperty *prop = object_property_find(obj, name);
-- 
1.7.7.6

[Qemu-devel] [PATCH 6/9] qdev: accept both strings and integers for PCI addresses

[Paolo Bonzini]

Visitors allow a limited form of polymorphism.  Exploit it to support
setting the non-legacy PCI address property both as a DD.F string
and as an 8-bit integer.

The 8-bit integer form is just too clumsy, it is unlikely that we will
ever drop it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/qdev-properties.c |   36 +++++++++++++++++++++++++-----------
 1 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index b6d6fcf..9a838b0 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -943,25 +943,40 @@ PropertyInfo qdev_prop_losttickpolicy = {
 /*
  * bus-local address, i.e. "$slot" or "$slot.$fn"
  */
-static int parse_pci_devfn(DeviceState *dev, Property *prop, const char *str)
+static void set_pci_devfn(Object *obj, Visitor *v, void *opaque,
+                          const char *name, Error **errp)
 {
+    DeviceState *dev = DEVICE(obj);
+    Property *prop = opaque;
     uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
     unsigned int slot, fn, n;
+    Error *local_err = NULL;
+    char *str = (char *)"";
+
+    if (dev->state != DEV_STATE_CREATED) {
+        error_set(errp, QERR_PERMISSION_DENIED);
+        return;
+    }
+
+    visit_type_str(v, &str, name, &local_err);
+    if (local_err) {
+        return set_int32(obj, v, opaque, name, errp);
+    }
 
     if (sscanf(str, "%x.%x%n", &slot, &fn, &n) != 2) {
         fn = 0;
         if (sscanf(str, "%x%n", &slot, &n) != 1) {
-            return -EINVAL;
+            goto invalid;
         }
     }
-    if (str[n] != '\0')
-        return -EINVAL;
-    if (fn > 7)
-        return -EINVAL;
-    if (slot > 31)
-        return -EINVAL;
+    if (str[n] != '\0' || fn > 7 || slot > 31) {
+        goto invalid;
+    }
     *ptr = slot << 3 | fn;
-    return 0;
+    return;
+
+invalid:
+    error_set_from_qdev_prop_error(errp, EINVAL, dev, prop, str);
 }
 
 static int print_pci_devfn(DeviceState *dev, Property *prop, char *dest, size_t len)
@@ -978,10 +993,9 @@ static int print_pci_devfn(DeviceState *dev, Property *prop, char *dest, size_t
 PropertyInfo qdev_prop_pci_devfn = {
     .name  = "int32",
     .legacy_name  = "pci-devfn",
-    .parse = parse_pci_devfn,
     .print = print_pci_devfn,
     .get   = get_int32,
-    .set   = set_int32,
+    .set   = set_pci_devfn,
     /* FIXME: this should be -1...255, but the address is stored
      * into an uint32_t rather than int32_t.
      */
-- 
1.7.7.6

[Qemu-devel] [PATCH 7/9] qdev: accept hex properties only if prefixed by 0x

[Paolo Bonzini]

Hex properties are an obstacle to removal of old qdev string parsing, but
even here we can lay down the foundations for future simplification.  In
general, they are rarely used and their printed form is more interesting
than the parsing.  For example you'd usually set isa-serial.index
instead of isa-serial.iobase.  And luckily our main client, libvirt
only cares about few of these, and always sets them with a 0x prefix.
So the series stops accepting bare hexadecimal numbers, preparing for
making legacy properties read-only in 1.3 or so.  The read side will
stay as long as "info qtree" is with us.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/qdev-properties.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 9a838b0..d47122a 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -164,6 +164,10 @@ static int parse_hex8(DeviceState *dev, Property *prop, const char *str)
     uint8_t *ptr = qdev_get_prop_ptr(dev, prop);
     char *end;
 
+    if (str[0] != '0' || str[1] != 'x') {
+        return -EINVAL;
+    }
+
     *ptr = strtoul(str, &end, 16);
     if ((*end != '\0') || (end == str)) {
         return -EINVAL;
@@ -369,6 +373,10 @@ static int parse_hex32(DeviceState *dev, Property *prop, const char *str)
     uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
     char *end;
 
+    if (str[0] != '0' || str[1] != 'x') {
+        return -EINVAL;
+    }
+
     *ptr = strtoul(str, &end, 16);
     if ((*end != '\0') || (end == str)) {
         return -EINVAL;
@@ -456,6 +464,10 @@ static int parse_hex64(DeviceState *dev, Property *prop, const char *str)
     uint64_t *ptr = qdev_get_prop_ptr(dev, prop);
     char *end;
 
+    if (str[0] != '0' || str[1] != 'x') {
+        return -EINVAL;
+    }
+
     *ptr = strtoull(str, &end, 16);
     if ((*end != '\0') || (end == str)) {
         return -EINVAL;
-- 
1.7.7.6

[Qemu-devel] [PATCH 8/9] qdev: use built-in QOM string parser

[Paolo Bonzini]

object_property_parse lets us drop the legacy setters when their task
is done just as well by the string visitors.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/qdev-properties.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index d47122a..97bda27 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -1080,9 +1080,9 @@ int qdev_prop_parse(DeviceState *dev, const char *name, const char *value)
 
     legacy_name = g_strdup_printf("legacy-%s", name);
     if (object_property_get_type(OBJECT(dev), legacy_name, NULL)) {
-        object_property_set_str(OBJECT(dev), value, legacy_name, &err);
+        object_property_parse(OBJECT(dev), value, legacy_name, &err);
     } else {
-        object_property_set_str(OBJECT(dev), value, name, &err);
+        object_property_parse(OBJECT(dev), value, name, &err);
     }
     g_free(legacy_name);
 
-- 
1.7.7.6

[Qemu-devel] [PATCH 9/9] qdev: drop unnecessary parse/print methods

[Paolo Bonzini]

More qdev printers could have been removed in the previous series, and
object_property_parse also made several parsers unnecessary.  In fact,
the new code is even more robust with respect to overflows, so clean
them up!

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/qdev-properties.c |  134 --------------------------------------------------
 1 files changed, 0 insertions(+), 134 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 97bda27..a96c628 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -27,16 +27,6 @@ static void bit_prop_set(DeviceState *dev, Property *props, bool val)
 }
 
 /* Bit */
-static int parse_bit(DeviceState *dev, Property *prop, const char *str)
-{
-    if (!strcasecmp(str, "on"))
-        bit_prop_set(dev, prop, true);
-    else if (!strcasecmp(str, "off"))
-        bit_prop_set(dev, prop, false);
-    else
-        return -EINVAL;
-    return 0;
-}
 
 static int print_bit(DeviceState *dev, Property *prop, char *dest, size_t len)
 {
@@ -79,7 +69,6 @@ static void set_bit(Object *obj, Visitor *v, void *opaque,
 PropertyInfo qdev_prop_bit = {
     .name  = "boolean",
     .legacy_name  = "on/off",
-    .parse = parse_bit,
     .print = print_bit,
     .get   = get_bit,
     .set   = set_bit,
@@ -87,26 +76,6 @@ PropertyInfo qdev_prop_bit = {
 
 /* --- 8bit integer --- */
 
-static int parse_uint8(DeviceState *dev, Property *prop, const char *str)
-{
-    uint8_t *ptr = qdev_get_prop_ptr(dev, prop);
-    char *end;
-
-    /* accept both hex and decimal */
-    *ptr = strtoul(str, &end, 0);
-    if ((*end != '\0') || (end == str)) {
-        return -EINVAL;
-    }
-
-    return 0;
-}
-
-static int print_uint8(DeviceState *dev, Property *prop, char *dest, size_t len)
-{
-    uint8_t *ptr = qdev_get_prop_ptr(dev, prop);
-    return snprintf(dest, len, "%" PRIu8, *ptr);
-}
-
 static void get_int8(Object *obj, Visitor *v, void *opaque,
                      const char *name, Error **errp)
 {
@@ -149,8 +118,6 @@ static void set_int8(Object *obj, Visitor *v, void *opaque,
 
 PropertyInfo qdev_prop_uint8 = {
     .name  = "uint8",
-    .parse = parse_uint8,
-    .print = print_uint8,
     .get   = get_int8,
     .set   = set_int8,
     .min   = 0,
@@ -195,26 +162,6 @@ PropertyInfo qdev_prop_hex8 = {
 
 /* --- 16bit integer --- */
 
-static int parse_uint16(DeviceState *dev, Property *prop, const char *str)
-{
-    uint16_t *ptr = qdev_get_prop_ptr(dev, prop);
-    char *end;
-
-    /* accept both hex and decimal */
-    *ptr = strtoul(str, &end, 0);
-    if ((*end != '\0') || (end == str)) {
-        return -EINVAL;
-    }
-
-    return 0;
-}
-
-static int print_uint16(DeviceState *dev, Property *prop, char *dest, size_t len)
-{
-    uint16_t *ptr = qdev_get_prop_ptr(dev, prop);
-    return snprintf(dest, len, "%" PRIu16, *ptr);
-}
-
 static void get_int16(Object *obj, Visitor *v, void *opaque,
                       const char *name, Error **errp)
 {
@@ -257,8 +204,6 @@ static void set_int16(Object *obj, Visitor *v, void *opaque,
 
 PropertyInfo qdev_prop_uint16 = {
     .name  = "uint16",
-    .parse = parse_uint16,
-    .print = print_uint16,
     .get   = get_int16,
     .set   = set_int16,
     .min   = 0,
@@ -267,26 +212,6 @@ PropertyInfo qdev_prop_uint16 = {
 
 /* --- 32bit integer --- */
 
-static int parse_uint32(DeviceState *dev, Property *prop, const char *str)
-{
-    uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
-    char *end;
-
-    /* accept both hex and decimal */
-    *ptr = strtoul(str, &end, 0);
-    if ((*end != '\0') || (end == str)) {
-        return -EINVAL;
-    }
-
-    return 0;
-}
-
-static int print_uint32(DeviceState *dev, Property *prop, char *dest, size_t len)
-{
-    uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
-    return snprintf(dest, len, "%" PRIu32, *ptr);
-}
-
 static void get_int32(Object *obj, Visitor *v, void *opaque,
                       const char *name, Error **errp)
 {
@@ -329,37 +254,14 @@ static void set_int32(Object *obj, Visitor *v, void *opaque,
 
 PropertyInfo qdev_prop_uint32 = {
     .name  = "uint32",
-    .parse = parse_uint32,
-    .print = print_uint32,
     .get   = get_int32,
     .set   = set_int32,
     .min   = 0,
     .max   = 0xFFFFFFFFULL,
 };
 
-static int parse_int32(DeviceState *dev, Property *prop, const char *str)
-{
-    int32_t *ptr = qdev_get_prop_ptr(dev, prop);
-    char *end;
-
-    *ptr = strtol(str, &end, 10);
-    if ((*end != '\0') || (end == str)) {
-        return -EINVAL;
-    }
-
-    return 0;
-}
-
-static int print_int32(DeviceState *dev, Property *prop, char *dest, size_t len)
-{
-    int32_t *ptr = qdev_get_prop_ptr(dev, prop);
-    return snprintf(dest, len, "%" PRId32, *ptr);
-}
-
 PropertyInfo qdev_prop_int32 = {
     .name  = "int32",
-    .parse = parse_int32,
-    .print = print_int32,
     .get   = get_int32,
     .set   = set_int32,
     .min   = -0x80000000LL,
@@ -404,26 +306,6 @@ PropertyInfo qdev_prop_hex32 = {
 
 /* --- 64bit integer --- */
 
-static int parse_uint64(DeviceState *dev, Property *prop, const char *str)
-{
-    uint64_t *ptr = qdev_get_prop_ptr(dev, prop);
-    char *end;
-
-    /* accept both hex and decimal */
-    *ptr = strtoull(str, &end, 0);
-    if ((*end != '\0') || (end == str)) {
-        return -EINVAL;
-    }
-
-    return 0;
-}
-
-static int print_uint64(DeviceState *dev, Property *prop, char *dest, size_t len)
-{
-    uint64_t *ptr = qdev_get_prop_ptr(dev, prop);
-    return snprintf(dest, len, "%" PRIu64, *ptr);
-}
-
 static void get_int64(Object *obj, Visitor *v, void *opaque,
                       const char *name, Error **errp)
 {
@@ -451,8 +333,6 @@ static void set_int64(Object *obj, Visitor *v, void *opaque,
 
 PropertyInfo qdev_prop_uint64 = {
     .name  = "uint64",
-    .parse = parse_uint64,
-    .print = print_uint64,
     .get   = get_int64,
     .set   = set_int64,
 };
@@ -749,19 +629,6 @@ PropertyInfo qdev_prop_netdev = {
 
 /* --- vlan --- */
 
-static int parse_vlan(DeviceState *dev, Property *prop, const char *str)
-{
-    VLANState **ptr = qdev_get_prop_ptr(dev, prop);
-    int id;
-
-    if (sscanf(str, "%d", &id) != 1)
-        return -EINVAL;
-    *ptr = qemu_find_vlan(id, 1);
-    if (*ptr == NULL)
-        return -ENOENT;
-    return 0;
-}
-
 static int print_vlan(DeviceState *dev, Property *prop, char *dest, size_t len)
 {
     VLANState **ptr = qdev_get_prop_ptr(dev, prop);
@@ -820,7 +687,6 @@ static void set_vlan(Object *obj, Visitor *v, void *opaque,
 
 PropertyInfo qdev_prop_vlan = {
     .name  = "vlan",
-    .parse = parse_vlan,
     .print = print_vlan,
     .get   = get_vlan,
     .set   = set_vlan,
-- 
1.7.7.6

[Qemu-devel] [PULL v2 0/9] qdev deconstruction, command-line episode

[Paolo Bonzini]

Anthony,

I'm sending a pull request since you informally said on IRC that you're
okay with the patches.

The following changes since commit 99c7f87826337fa81f2f0f9baa9ca0a44faf90e9:

  input: send kbd+mouse events only to running guests. (2012-02-17 11:02:55 -0600)

are available in the git repository at:
  git://github.com/bonzini/qemu.git qdev-props-for-anthony

v1->v2:
	Fix bug in parsing booleans and strengthen test suite.
	The interdiff is attached.

Paolo Bonzini (9):
      qapi: allow sharing enum implementation across visitors
      qapi: drop qmp_input_end_optional
      qapi: add string-based visitors
      qapi: add tests for string-based visitors
      qom: add generic string parsing/printing
      qdev: accept both strings and integers for PCI addresses
      qdev: accept hex properties only if prefixed by 0x
      qdev: use built-in QOM string parser
      qdev: drop unnecessary parse/print methods

 .gitignore                   |    2 +
 Makefile.objs                |    5 +-
 hw/qdev-properties.c         |  186 +++++++++-------------------------------
 include/qemu/object.h        |   24 +++++
 qapi/qapi-visit-core.c       |   51 +++++++++++
 qapi/qapi-visit-impl.h       |   23 +++++
 qapi/qmp-input-visitor.c     |   39 +--------
 qapi/qmp-output-visitor.c    |   22 +-----
 qapi/string-input-visitor.c  |  138 +++++++++++++++++++++++++++++
 qapi/string-input-visitor.h  |   25 ++++++
 qapi/string-output-visitor.c |   89 +++++++++++++++++++
 qapi/string-output-visitor.h |   26 ++++++
 qom/object.c                 |   24 +++++
 test-string-input-visitor.c  |  195 ++++++++++++++++++++++++++++++++++++++++++
 test-string-output-visitor.c |  188 ++++++++++++++++++++++++++++++++++++++++
 tests/Makefile               |   12 ++-
 16 files changed, 841 insertions(+), 208 deletions(-)
 create mode 100644 qapi/qapi-visit-impl.h
 create mode 100644 qapi/string-input-visitor.c
 create mode 100644 qapi/string-input-visitor.h
 create mode 100644 qapi/string-output-visitor.c
 create mode 100644 qapi/string-output-visitor.h
 create mode 100644 test-string-input-visitor.c
 create mode 100644 test-string-output-visitor.c

diff --git a/include/qemu/object.h b/include/qemu/object.h
index 2081ca0..ff6be14 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -742,7 +742,7 @@ void object_property_parse(Object *obj, const char *string,
                            const char *name, struct Error **errp);
 
 /**
- * object_property_set:
+ * object_property_print:
  * @obj: the object
  * @name: the name of the property
  * @errp: returns an error if this function fails
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index ceee699..497eb9a 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -47,13 +47,15 @@ static void parse_type_bool(Visitor *v, bool *obj, const char *name,
     StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
 
     if (siv->string) {
-        if (strcasecmp(siv->string, "on") || strcasecmp(siv->string, "yes") ||
-            strcasecmp(siv->string, "true")) {
+        if (!strcasecmp(siv->string, "on") ||
+            !strcasecmp(siv->string, "yes") ||
+            !strcasecmp(siv->string, "true")) {
             *obj = true;
             return;
         }
-        if (strcasecmp(siv->string, "off") || strcasecmp(siv->string, "no") ||
-            strcasecmp(siv->string, "false")) {
+        if (!strcasecmp(siv->string, "off") ||
+            !strcasecmp(siv->string, "no") ||
+            !strcasecmp(siv->string, "false")) {
             *obj = false;
             return;
         }
diff --git a/test-string-input-visitor.c b/test-string-input-visitor.c
index ba2cc40..5370e32 100644
--- a/test-string-input-visitor.c
+++ b/test-string-input-visitor.c
@@ -75,6 +75,41 @@ static void test_visitor_in_bool(TestInputVisitorData *data,
     visit_type_bool(v, &res, NULL, &errp);
     g_assert(!error_is_set(&errp));
     g_assert_cmpint(res, ==, true);
+    visitor_input_teardown(data, unused);
+
+    v = visitor_input_test_init(data, "yes");
+
+    visit_type_bool(v, &res, NULL, &errp);
+    g_assert(!error_is_set(&errp));
+    g_assert_cmpint(res, ==, true);
+    visitor_input_teardown(data, unused);
+
+    v = visitor_input_test_init(data, "on");
+
+    visit_type_bool(v, &res, NULL, &errp);
+    g_assert(!error_is_set(&errp));
+    g_assert_cmpint(res, ==, true);
+    visitor_input_teardown(data, unused);
+
+    v = visitor_input_test_init(data, "false");
+
+    visit_type_bool(v, &res, NULL, &errp);
+    g_assert(!error_is_set(&errp));
+    g_assert_cmpint(res, ==, false);
+    visitor_input_teardown(data, unused);
+
+    v = visitor_input_test_init(data, "no");
+
+    visit_type_bool(v, &res, NULL, &errp);
+    g_assert(!error_is_set(&errp));
+    g_assert_cmpint(res, ==, false);
+    visitor_input_teardown(data, unused);
+
+    v = visitor_input_test_init(data, "off");
+
+    visit_type_bool(v, &res, NULL, &errp);
+    g_assert(!error_is_set(&errp));
+    g_assert_cmpint(res, ==, false);
 }
 
 static void test_visitor_in_number(TestInputVisitorData *data,
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH 1/9] qapi: allow sharing enum implementation across visitors

[Andreas Färber]

Am 09.02.2012 15:31, schrieb Paolo Bonzini:
> Most visitors will use the same code for enum parsing.  Move it to
> the core.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Andreas Färber <afaerber@suse.de>

However...

> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> index ddef3ed..a4e088c 100644
> --- a/qapi/qapi-visit-core.c
> +++ b/qapi/qapi-visit-core.c

> @@ -116,3 +117,53 @@ void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp)
>          v->type_number(v, obj, name, errp);
>      }
>  }
> +
> +void output_type_enum(Visitor *v, int *obj, const char *strings[],
> +                      const char *kind, const char *name,
> +                      Error **errp)
> +{
> +    int i = 0;
> +    int value = *obj;
> +    char *enum_str;
> +
> +    assert(strings);
> +    while (strings[i++] != NULL);
> +    if (value < 0 || value >= i - 1) {
> +        error_set(errp, QERR_INVALID_PARAMETER, name ? name : "null");
> +        return;
> +    }
> +
> +    enum_str = (char *)strings[value];

This does not take into account non-linear enum values.

Maybe name it output_type_linear_enum to allow for alternative enum
lookup implementations? (e.g., hashtable or list of name,value tuples)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 5/9] qom: add generic string parsing/printing

[Andreas Färber]

Am 09.02.2012 15:31, schrieb Paolo Bonzini:
> Add generic property accessors that take a string and parse it
> appropriately for the property type.  All the magic here is done
> by the new string-based visitors.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/qemu/object.h |   24 ++++++++++++++++++++++++
>  qom/object.c          |   24 ++++++++++++++++++++++++
>  2 files changed, 48 insertions(+), 0 deletions(-)
> 
> diff --git a/include/qemu/object.h b/include/qemu/object.h
> index 7d50da9..af071eb 100644
> --- a/include/qemu/object.h
> +++ b/include/qemu/object.h
> @@ -730,6 +730,30 @@ void object_property_set(Object *obj, struct Visitor *v, const char *name,
>                           struct Error **errp);
>  
>  /**
> + * object_property_parse:
> + * @obj: the object
> + * @string: the string that will be used to parse the property value.
> + * @name: the name of the property
> + * @errp: returns an error if this function fails
> + *
> + * Parses a string and writes the result into a property of an object.
> + */
> +void object_property_parse(Object *obj, const char *string,
> +                           const char *name, struct Error **errp);
> +
> +/**
> + * object_property_set:
> + * @obj: the object
> + * @name: the name of the property
> + * @errp: returns an error if this function fails
> + *
> + * Returns a string representation of the value of the property.  The

 * Returns: for gtk-doc.

> + * caller shall free the string.
> + */
> +char *object_property_print(Object *obj, const char *name,
> +                            struct Error **errp);
> +
> +/**
>   * @object_property_get_type:

(This non-PULL version will conflict here.)

>   * @obj: the object
>   * @name: the name of the property
> diff --git a/qom/object.c b/qom/object.c
> index 5e5b261..97d898f 100644
> --- a/qom/object.c
> +++ b/qom/object.c

> @@ -782,6 +784,28 @@ int64_t object_property_get_int(Object *obj, const char *name,
>      return retval;
>  }
>  
> +void object_property_parse(Object *obj, const char *string,
> +                           const char *name, Error **errp)
> +{
> +    StringInputVisitor *mi;

Curious: where does mi/mo come from?

> +    mi = string_input_visitor_new(string);
> +    object_property_set(obj, string_input_get_visitor(mi), name, errp);
> +
> +    string_input_visitor_cleanup(mi);
> +}
> +
> +char *object_property_print(Object *obj, const char *name,
> +                            Error **errp)
> +{
> +    StringOutputVisitor *mo;
> +    char *string;
> +
> +    mo = string_output_visitor_new();
> +    object_property_get(obj, string_output_get_visitor(mo), name, NULL);
> +    string = string_output_get_string(mo);
> +    string_output_visitor_cleanup(mo);
> +    return string;
> +}

Empty line please. Otherwise Liked-by: Andreas Färber <afaerber@suse.de>

>  const char *object_property_get_type(Object *obj, const char *name, Error **errp)
>  {
>      ObjectProperty *prop = object_property_find(obj, name);

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 1/9] qapi: allow sharing enum implementation across visitors

[Paolo Bonzini]

On 02/21/2012 09:31 PM, Andreas Färber wrote:
>> > +void output_type_enum(Visitor *v, int *obj, const char *strings[],
>> > +                      const char *kind, const char *name,
>> > +                      Error **errp)
>> > +{
>> > +    int i = 0;
>> > +    int value = *obj;
>> > +    char *enum_str;
>> > +
>> > +    assert(strings);
>> > +    while (strings[i++] != NULL);
>> > +    if (value < 0 || value >= i - 1) {
>> > +        error_set(errp, QERR_INVALID_PARAMETER, name ? name : "null");
>> > +        return;
>> > +    }
>> > +
>> > +    enum_str = (char *)strings[value];
> This does not take into account non-linear enum values.
> 
> Maybe name it output_type_linear_enum to allow for alternative enum
> lookup implementations? (e.g., hashtable or list of name,value tuples)

I would say that this is the common case and the other one should be
named output_type_sparse_enum or something like that, if the need arises...

Paolo

Re: [Qemu-devel] [PATCH 5/9] qom: add generic string parsing/printing

[Paolo Bonzini]

On 02/21/2012 09:47 PM, Andreas Färber wrote:
>> > +void object_property_parse(Object *obj, const char *string,
>> > +                           const char *name, Error **errp)
>> > +{
>> > +    StringInputVisitor *mi;
> Curious: where does mi/mo come from?

Cut-and-paste from the version using QMP visitors.

>> > +    mi = string_input_visitor_new(string);
>> > +    object_property_set(obj, string_input_get_visitor(mi), name, errp);
>> > +
>> > +    string_input_visitor_cleanup(mi);
>> > +}
>> > +
>> > +char *object_property_print(Object *obj, const char *name,
>> > +                            Error **errp)
>> > +{
>> > +    StringOutputVisitor *mo;
>> > +    char *string;
>> > +
>> > +    mo = string_output_visitor_new();
>> > +    object_property_get(obj, string_output_get_visitor(mo), name, NULL);
>> > +    string = string_output_get_string(mo);
>> > +    string_output_visitor_cleanup(mo);
>> > +    return string;
>> > +}

Paolo

Re: [Qemu-devel] [PULL v2 0/9] qdev deconstruction, command-line episode

[Anthony Liguori]

On 02/21/2012 11:13 AM, Paolo Bonzini wrote:
> Anthony,
>
> I'm sending a pull request since you informally said on IRC that you're
> okay with the patches.
>
> The following changes since commit 99c7f87826337fa81f2f0f9baa9ca0a44faf90e9:
>
>    input: send kbd+mouse events only to running guests. (2012-02-17 11:02:55 -0600)

Pulled.  Thanks.

Regards,

Anthony Liguori

>
> are available in the git repository at:
>    git://github.com/bonzini/qemu.git qdev-props-for-anthony
>
> v1->v2:
> 	Fix bug in parsing booleans and strengthen test suite.
> 	The interdiff is attached.
>
> Paolo Bonzini (9):
>        qapi: allow sharing enum implementation across visitors
>        qapi: drop qmp_input_end_optional
>        qapi: add string-based visitors
>        qapi: add tests for string-based visitors
>        qom: add generic string parsing/printing
>        qdev: accept both strings and integers for PCI addresses
>        qdev: accept hex properties only if prefixed by 0x
>        qdev: use built-in QOM string parser
>        qdev: drop unnecessary parse/print methods
>
>   .gitignore                   |    2 +
>   Makefile.objs                |    5 +-
>   hw/qdev-properties.c         |  186 +++++++++-------------------------------
>   include/qemu/object.h        |   24 +++++
>   qapi/qapi-visit-core.c       |   51 +++++++++++
>   qapi/qapi-visit-impl.h       |   23 +++++
>   qapi/qmp-input-visitor.c     |   39 +--------
>   qapi/qmp-output-visitor.c    |   22 +-----
>   qapi/string-input-visitor.c  |  138 +++++++++++++++++++++++++++++
>   qapi/string-input-visitor.h  |   25 ++++++
>   qapi/string-output-visitor.c |   89 +++++++++++++++++++
>   qapi/string-output-visitor.h |   26 ++++++
>   qom/object.c                 |   24 +++++
>   test-string-input-visitor.c  |  195 ++++++++++++++++++++++++++++++++++++++++++
>   test-string-output-visitor.c |  188 ++++++++++++++++++++++++++++++++++++++++
>   tests/Makefile               |   12 ++-
>   16 files changed, 841 insertions(+), 208 deletions(-)
>   create mode 100644 qapi/qapi-visit-impl.h
>   create mode 100644 qapi/string-input-visitor.c
>   create mode 100644 qapi/string-input-visitor.h
>   create mode 100644 qapi/string-output-visitor.c
>   create mode 100644 qapi/string-output-visitor.h
>   create mode 100644 test-string-input-visitor.c
>   create mode 100644 test-string-output-visitor.c
>
> diff --git a/include/qemu/object.h b/include/qemu/object.h
> index 2081ca0..ff6be14 100644
> --- a/include/qemu/object.h
> +++ b/include/qemu/object.h
> @@ -742,7 +742,7 @@ void object_property_parse(Object *obj, const char *string,
>                              const char *name, struct Error **errp);
>
>   /**
> - * object_property_set:
> + * object_property_print:
>    * @obj: the object
>    * @name: the name of the property
>    * @errp: returns an error if this function fails
> diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> index ceee699..497eb9a 100644
> --- a/qapi/string-input-visitor.c
> +++ b/qapi/string-input-visitor.c
> @@ -47,13 +47,15 @@ static void parse_type_bool(Visitor *v, bool *obj, const char *name,
>       StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
>
>       if (siv->string) {
> -        if (strcasecmp(siv->string, "on") || strcasecmp(siv->string, "yes") ||
> -            strcasecmp(siv->string, "true")) {
> +        if (!strcasecmp(siv->string, "on") ||
> +            !strcasecmp(siv->string, "yes") ||
> +            !strcasecmp(siv->string, "true")) {
>               *obj = true;
>               return;
>           }
> -        if (strcasecmp(siv->string, "off") || strcasecmp(siv->string, "no") ||
> -            strcasecmp(siv->string, "false")) {
> +        if (!strcasecmp(siv->string, "off") ||
> +            !strcasecmp(siv->string, "no") ||
> +            !strcasecmp(siv->string, "false")) {
>               *obj = false;
>               return;
>           }
> diff --git a/test-string-input-visitor.c b/test-string-input-visitor.c
> index ba2cc40..5370e32 100644
> --- a/test-string-input-visitor.c
> +++ b/test-string-input-visitor.c
> @@ -75,6 +75,41 @@ static void test_visitor_in_bool(TestInputVisitorData *data,
>       visit_type_bool(v,&res, NULL,&errp);
>       g_assert(!error_is_set(&errp));
>       g_assert_cmpint(res, ==, true);
> +    visitor_input_teardown(data, unused);
> +
> +    v = visitor_input_test_init(data, "yes");
> +
> +    visit_type_bool(v,&res, NULL,&errp);
> +    g_assert(!error_is_set(&errp));
> +    g_assert_cmpint(res, ==, true);
> +    visitor_input_teardown(data, unused);
> +
> +    v = visitor_input_test_init(data, "on");
> +
> +    visit_type_bool(v,&res, NULL,&errp);
> +    g_assert(!error_is_set(&errp));
> +    g_assert_cmpint(res, ==, true);
> +    visitor_input_teardown(data, unused);
> +
> +    v = visitor_input_test_init(data, "false");
> +
> +    visit_type_bool(v,&res, NULL,&errp);
> +    g_assert(!error_is_set(&errp));
> +    g_assert_cmpint(res, ==, false);
> +    visitor_input_teardown(data, unused);
> +
> +    v = visitor_input_test_init(data, "no");
> +
> +    visit_type_bool(v,&res, NULL,&errp);
> +    g_assert(!error_is_set(&errp));
> +    g_assert_cmpint(res, ==, false);
> +    visitor_input_teardown(data, unused);
> +
> +    v = visitor_input_test_init(data, "off");
> +
> +    visit_type_bool(v,&res, NULL,&errp);
> +    g_assert(!error_is_set(&errp));
> +    g_assert_cmpint(res, ==, false);
>   }
>
>   static void test_visitor_in_number(TestInputVisitorData *data,