qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/3] add fixed-width visitors and serialization tests
@ 2012-02-22 19:00 Michael Roth
  2012-02-22 19:00 ` [Qemu-devel] [PATCH 1/3] qapi: add Visitor interfaces for uint*_t and int*_t Michael Roth
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Michael Roth @ 2012-02-22 19:00 UTC (permalink / raw)
  To: qemu-devel

These patches apply on top of qemu.git master, and can also be obtained from:
git://github.com/mdroth/qemu.git visitor-fixed-width-v1

These patches add fixed-width visitor types and switches all qdev users over to
them.

We also add a test suite which covers these as well as does some sanity
checking on Visitors which may be used in the future for serialization-type
use-cases.

I've also tried plugging in the new String-based visitors, but hit some test
failures due to how they handle floating point so I've held off on those while
I look into it more. That can be done in seperate patch though.

 hw/mc146818rtc.c             |    7 -
 hw/qdev-addr.c               |    4 +-
 hw/qdev-properties.c         |   42 +--
 qapi/qapi-visit-core.c       |  139 ++++++++
 qapi/qapi-visit-core.h       |   16 +
 test-visitor-serialization.c |  718 ++++++++++++++++++++++++++++++++++++++++++
 tests/Makefile               |    6 +-
 7 files changed, 897 insertions(+), 35 deletions(-)

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Qemu-devel] [PATCH 1/3] qapi: add Visitor interfaces for uint*_t and int*_t
  2012-02-22 19:00 [Qemu-devel] [PATCH 0/3] add fixed-width visitors and serialization tests Michael Roth
@ 2012-02-22 19:00 ` Michael Roth
  2012-02-22 19:00 ` [Qemu-devel] [PATCH 2/3] qapi: unit tests for visitor-based serialization Michael Roth
  2012-02-22 19:00 ` [Qemu-devel] [PATCH 3/3] qdev: switch property accessors to fixed-width visitor interfaces Michael Roth
  2 siblings, 0 replies; 4+ messages in thread
From: Michael Roth @ 2012-02-22 19:00 UTC (permalink / raw)
  To: qemu-devel

This adds visitor interfaces for fixed-width integers types.
Implementing these in visitors is optional, otherwise we fall back to
visit_type_int() (int64_t) with some additional bounds checking to avoid
integer overflows for cases where the value fetched exceeds the bounds
of our target C type.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 hw/mc146818rtc.c       |    7 ---
 qapi/qapi-visit-core.c |  139 ++++++++++++++++++++++++++++++++++++++++++++++++
 qapi/qapi-visit-core.h |   16 ++++++
 3 files changed, 155 insertions(+), 7 deletions(-)

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 6c1ad38..a5d45d4 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -621,13 +621,6 @@ static const MemoryRegionOps cmos_ops = {
     .old_portio = cmos_portio
 };
 
-// FIXME add int32 visitor
-static void visit_type_int32(Visitor *v, int *value, const char *name, Error **errp)
-{
-    int64_t val = *value;
-    visit_type_int(v, &val, name, errp);
-}
-
 static void rtc_get_date(Object *obj, Visitor *v, void *opaque,
                          const char *name, Error **errp)
 {
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index a4e088c..6823e84 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -97,6 +97,145 @@ void visit_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
     }
 }
 
+void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error **errp)
+{
+    int64_t value;
+    if (!error_is_set(errp)) {
+        if (v->type_uint8) {
+            v->type_uint8(v, obj, name, errp);
+        } else {
+            value = *obj;
+            v->type_int(v, &value, name, errp);
+            if (value > UINT8_MAX) {
+                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+                          "uint8_t");
+                return;
+            }
+            *obj = value;
+        }
+    }
+}
+
+void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error **errp)
+{
+    int64_t value;
+    if (!error_is_set(errp)) {
+        if (v->type_uint16) {
+            v->type_uint16(v, obj, name, errp);
+        } else {
+            value = *obj;
+            v->type_int(v, &value, name, errp);
+            if (value > UINT16_MAX) {
+                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+                          "uint16_t");
+                return;
+            }
+            *obj = value;
+        }
+    }
+}
+
+void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name, Error **errp)
+{
+    int64_t value;
+    if (!error_is_set(errp)) {
+        if (v->type_uint32) {
+            v->type_uint32(v, obj, name, errp);
+        } else {
+            value = *obj;
+            v->type_int(v, &value, name, errp);
+            if (value > UINT32_MAX) {
+                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+                          "uint32_t");
+                return;
+            }
+            *obj = value;
+        }
+    }
+}
+
+void visit_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
+{
+    int64_t value;
+    if (!error_is_set(errp)) {
+        if (v->type_uint64) {
+            v->type_uint64(v, obj, name, errp);
+        } else {
+            value = *obj;
+            v->type_int(v, &value, name, errp);
+            *obj = value;
+        }
+    }
+}
+
+void visit_type_int8(Visitor *v, int8_t *obj, const char *name, Error **errp)
+{
+    int64_t value;
+    if (!error_is_set(errp)) {
+        if (v->type_int8) {
+            v->type_int8(v, obj, name, errp);
+        } else {
+            value = *obj;
+            v->type_int(v, &value, name, errp);
+            if (value < INT8_MIN || value > INT8_MAX) {
+                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+                          "uint8_t");
+                return;
+            }
+            *obj = value;
+        }
+    }
+}
+
+void visit_type_int16(Visitor *v, int16_t *obj, const char *name, Error **errp)
+{
+    int64_t value;
+    if (!error_is_set(errp)) {
+        if (v->type_int16) {
+            v->type_int16(v, obj, name, errp);
+        } else {
+            value = *obj;
+            v->type_int(v, &value, name, errp);
+            if (value < INT16_MIN || value > INT16_MAX) {
+                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+                          "uint16_t");
+                return;
+            }
+            *obj = value;
+        }
+    }
+}
+
+void visit_type_int32(Visitor *v, int32_t *obj, const char *name, Error **errp)
+{
+    int64_t value;
+    if (!error_is_set(errp)) {
+        if (v->type_int32) {
+            v->type_int32(v, obj, name, errp);
+        } else {
+            value = *obj;
+            v->type_int(v, &value, name, errp);
+            if (value < INT32_MIN || value > INT32_MAX) {
+                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+                          "uint32_t");
+                return;
+            }
+            *obj = value;
+        }
+    }
+}
+
+void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp)
+{
+    if (!error_is_set(errp)) {
+        if (v->type_int64) {
+            v->type_int64(v, obj, name, errp);
+        } else {
+            v->type_int(v, obj, name, errp);
+        }
+    }
+}
+
 void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp)
 {
     if (!error_is_set(errp)) {
diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
index e850746..a19d70c 100644
--- a/qapi/qapi-visit-core.h
+++ b/qapi/qapi-visit-core.h
@@ -52,6 +52,14 @@ struct Visitor
     void (*start_handle)(Visitor *v, void **obj, const char *kind,
                          const char *name, Error **errp);
     void (*end_handle)(Visitor *v, Error **errp);
+    void (*type_uint8)(Visitor *v, uint8_t *obj, const char *name, Error **errp);
+    void (*type_uint16)(Visitor *v, uint16_t *obj, const char *name, Error **errp);
+    void (*type_uint32)(Visitor *v, uint32_t *obj, const char *name, Error **errp);
+    void (*type_uint64)(Visitor *v, uint64_t *obj, const char *name, Error **errp);
+    void (*type_int8)(Visitor *v, int8_t *obj, const char *name, Error **errp);
+    void (*type_int16)(Visitor *v, int16_t *obj, const char *name, Error **errp);
+    void (*type_int32)(Visitor *v, int32_t *obj, const char *name, Error **errp);
+    void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error **errp);
 };
 
 void visit_start_handle(Visitor *v, void **obj, const char *kind,
@@ -69,6 +77,14 @@ void visit_end_optional(Visitor *v, Error **errp);
 void visit_type_enum(Visitor *v, int *obj, const char *strings[],
                      const char *kind, const char *name, Error **errp);
 void visit_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp);
+void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error **errp);
+void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error **errp);
+void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name, Error **errp);
+void visit_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp);
+void visit_type_int8(Visitor *v, int8_t *obj, const char *name, Error **errp);
+void visit_type_int16(Visitor *v, int16_t *obj, const char *name, Error **errp);
+void visit_type_int32(Visitor *v, int32_t *obj, const char *name, Error **errp);
+void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp);
 void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
 void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
 void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [Qemu-devel] [PATCH 2/3] qapi: unit tests for visitor-based serialization
  2012-02-22 19:00 [Qemu-devel] [PATCH 0/3] add fixed-width visitors and serialization tests Michael Roth
  2012-02-22 19:00 ` [Qemu-devel] [PATCH 1/3] qapi: add Visitor interfaces for uint*_t and int*_t Michael Roth
@ 2012-02-22 19:00 ` Michael Roth
  2012-02-22 19:00 ` [Qemu-devel] [PATCH 3/3] qdev: switch property accessors to fixed-width visitor interfaces Michael Roth
  2 siblings, 0 replies; 4+ messages in thread
From: Michael Roth @ 2012-02-22 19:00 UTC (permalink / raw)
  To: qemu-devel

Currently we test our visitors individually, and seperately for input
vs. output. This is useful for validating internal representations
against the native C types and vice-versa, and other visitor-specific
testing, but it doesn't cover the potential use-case of using visitor
pairs for serialization/deserialization very well, and makes it
hard to easily extend the coverage for different C types / boundary
conditions.

To cover that we add a set of unit tests that takes a number of native C
values, passes them into an output visitor, extracts the values with an
input visitor, then compares the result to the original.

Plugging in new visitors to the test harness only requires a user to
implement the SerializeOps interface and add it to a list.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 test-visitor-serialization.c |  718 ++++++++++++++++++++++++++++++++++++++++++
 tests/Makefile               |    6 +-
 2 files changed, 723 insertions(+), 1 deletions(-)
 create mode 100644 test-visitor-serialization.c

diff --git a/test-visitor-serialization.c b/test-visitor-serialization.c
new file mode 100644
index 0000000..4e428da
--- /dev/null
+++ b/test-visitor-serialization.c
@@ -0,0 +1,718 @@
+/*
+ * Unit-tests for visitor-based serialization
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Michael Roth <mdroth@linux.vnet.ibm.com>
+ *
+ * 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 <stdlib.h>
+#include <stdint.h>
+#include <float.h>
+#include "test-qapi-types.h"
+#include "test-qapi-visit.h"
+#include "qemu-objects.h"
+#include "qapi/qmp-input-visitor.h"
+#include "qapi/qmp-output-visitor.h"
+
+typedef struct PrimitiveType {
+    union {
+        const char *string;
+        bool boolean;
+        double number;
+        int64_t integer;
+        uint8_t u8;
+        uint16_t u16;
+        uint32_t u32;
+        uint64_t u64;
+        int8_t s8;
+        int16_t s16;
+        int32_t s32;
+        int64_t s64;
+        intmax_t max;
+    } value;
+    enum {
+        PTYPE_STRING = 0,
+        PTYPE_BOOLEAN,
+        PTYPE_NUMBER,
+        PTYPE_INTEGER,
+        PTYPE_U8,
+        PTYPE_U16,
+        PTYPE_U32,
+        PTYPE_U64,
+        PTYPE_S8,
+        PTYPE_S16,
+        PTYPE_S32,
+        PTYPE_S64,
+        PTYPE_EOL,
+    } type;
+    const char *description;
+} PrimitiveType;
+
+/* test helpers */
+
+static void visit_primitive_type(Visitor *v, void **native, Error **errp)
+{
+    PrimitiveType *pt = *native;
+    switch(pt->type) {
+    case PTYPE_STRING:
+        visit_type_str(v, (char **)&pt->value.string, NULL, errp);
+        break;
+    case PTYPE_BOOLEAN:
+        visit_type_bool(v, &pt->value.boolean, NULL, errp);
+        break;
+    case PTYPE_NUMBER:
+        visit_type_number(v, &pt->value.number, NULL, errp);
+        break;
+    case PTYPE_INTEGER:
+        visit_type_int(v, &pt->value.integer, NULL, errp);
+        break;
+    case PTYPE_U8:
+        visit_type_uint8(v, &pt->value.u8, NULL, errp);
+        break;
+    case PTYPE_U16:
+        visit_type_uint16(v, &pt->value.u16, NULL, errp);
+        break;
+    case PTYPE_U32:
+        visit_type_uint32(v, &pt->value.u32, NULL, errp);
+        break;
+    case PTYPE_U64:
+        visit_type_uint64(v, &pt->value.u64, NULL, errp);
+        break;
+    case PTYPE_S8:
+        visit_type_int8(v, &pt->value.s8, NULL, errp);
+        break;
+    case PTYPE_S16:
+        visit_type_int16(v, &pt->value.s16, NULL, errp);
+        break;
+    case PTYPE_S32:
+        visit_type_int32(v, &pt->value.s32, NULL, errp);
+        break;
+    case PTYPE_S64:
+        visit_type_int64(v, &pt->value.s64, NULL, errp);
+        break;
+    case PTYPE_EOL:
+        g_assert(false);
+    }
+}
+
+typedef struct TestStruct
+{
+    int64_t integer;
+    bool boolean;
+    char *string;
+} TestStruct;
+
+static void visit_type_TestStruct(Visitor *v, TestStruct **obj,
+                                  const char *name, Error **errp)
+{
+    visit_start_struct(v, (void **)obj, NULL, name, sizeof(TestStruct), errp);
+
+    visit_type_int(v, &(*obj)->integer, "integer", errp);
+    visit_type_bool(v, &(*obj)->boolean, "boolean", errp);
+    visit_type_str(v, &(*obj)->string, "string", errp);
+
+    visit_end_struct(v, errp);
+}
+
+static TestStruct *struct_create(void)
+{
+    TestStruct *ts = g_malloc0(sizeof(*ts));
+    ts->integer = -42;
+    ts->boolean = true;
+    ts->string = strdup("test string");
+    return ts;
+}
+
+static void struct_compare(TestStruct *ts1, TestStruct *ts2)
+{
+    g_assert(ts1);
+    g_assert(ts2);
+    g_assert_cmpint(ts1->integer, ==, ts2->integer);
+    g_assert(ts1->boolean == ts2->boolean);
+    g_assert_cmpstr(ts1->string, ==, ts2->string);
+}
+
+static void struct_cleanup(TestStruct *ts)
+{
+    g_free(ts->string);
+    g_free(ts);
+}
+
+static void visit_struct(Visitor *v, void **native, Error **errp)
+{
+    visit_type_TestStruct(v, (TestStruct **)native, NULL, errp);
+}
+
+static UserDefNested *nested_struct_create(void)
+{
+    UserDefNested *udnp = g_malloc0(sizeof(*udnp));
+    udnp->string0 = strdup("test_string0");
+    udnp->dict1.string1 = strdup("test_string1");
+    udnp->dict1.dict2.userdef1 = g_malloc0(sizeof(UserDefOne));
+    udnp->dict1.dict2.userdef1->integer = 42;
+    udnp->dict1.dict2.userdef1->string = strdup("test_string");
+    udnp->dict1.dict2.string2 = strdup("test_string2");
+    udnp->dict1.has_dict3 = true;
+    udnp->dict1.dict3.userdef2 = g_malloc0(sizeof(UserDefOne));
+    udnp->dict1.dict3.userdef2->integer = 43;
+    udnp->dict1.dict3.userdef2->string = strdup("test_string");
+    udnp->dict1.dict3.string3 = strdup("test_string3");
+    return udnp;
+}
+
+static void nested_struct_compare(UserDefNested *udnp1, UserDefNested *udnp2)
+{
+    g_assert(udnp1);
+    g_assert(udnp2);
+    g_assert_cmpstr(udnp1->string0, ==, udnp2->string0);
+    g_assert_cmpstr(udnp1->dict1.string1, ==, udnp2->dict1.string1);
+    g_assert_cmpint(udnp1->dict1.dict2.userdef1->integer, ==,
+                    udnp2->dict1.dict2.userdef1->integer);
+    g_assert_cmpstr(udnp1->dict1.dict2.userdef1->string, ==,
+                    udnp2->dict1.dict2.userdef1->string);
+    g_assert_cmpstr(udnp1->dict1.dict2.string2, ==, udnp2->dict1.dict2.string2);
+    g_assert(udnp1->dict1.has_dict3 == udnp2->dict1.has_dict3);
+    g_assert_cmpint(udnp1->dict1.dict3.userdef2->integer, ==,
+                    udnp2->dict1.dict3.userdef2->integer);
+    g_assert_cmpstr(udnp1->dict1.dict3.userdef2->string, ==,
+                    udnp2->dict1.dict3.userdef2->string);
+    g_assert_cmpstr(udnp1->dict1.dict3.string3, ==, udnp2->dict1.dict3.string3);
+}
+
+static void nested_struct_cleanup(UserDefNested *udnp)
+{
+    qapi_free_UserDefNested(udnp);
+}
+
+static void visit_nested_struct(Visitor *v, void **native, Error **errp)
+{
+    visit_type_UserDefNested(v, (UserDefNested **)native, NULL, errp);
+}
+
+static void visit_nested_struct_list(Visitor *v, void **native, Error **errp)
+{
+    visit_type_UserDefNestedList(v, (UserDefNestedList **)native, NULL, errp);
+}
+
+/* test cases */
+
+typedef void (*VisitorFunc)(Visitor *v, void **native, Error **errp);
+
+typedef enum VisitorCapabilities {
+    VCAP_PRIMITIVES = 1,
+    VCAP_STRUCTURES = 2,
+    VCAP_LISTS = 4,
+} VisitorCapabilities;
+
+typedef struct SerializeOps {
+    void (*serialize)(void *native_in, void **datap,
+                      VisitorFunc visit, Error **errp);
+    void (*deserialize)(void **native_out, void *datap,
+                            VisitorFunc visit, Error **errp);
+    void (*cleanup)(void *datap);
+    const char *type;
+    VisitorCapabilities caps;
+} SerializeOps;
+
+typedef struct TestArgs {
+    const SerializeOps *ops;
+    void *test_data;
+} TestArgs;
+
+static void test_primitives(gconstpointer opaque)
+{
+    TestArgs *args = (TestArgs *) opaque;
+    const SerializeOps *ops = args->ops;
+    PrimitiveType *pt = args->test_data;
+    PrimitiveType *pt_copy = g_malloc0(sizeof(*pt_copy));
+    Error *err = NULL;
+    void *serialize_data;
+
+    pt_copy->type = pt->type;
+    ops->serialize(pt, &serialize_data, visit_primitive_type, &err);
+    ops->deserialize((void **)&pt_copy, serialize_data, visit_primitive_type, &err);
+
+    g_assert(err == NULL);
+    g_assert(pt_copy != NULL);
+    if (pt->type == PTYPE_STRING) {
+        g_assert_cmpstr(pt->value.string, ==, pt_copy->value.string);
+    } else if (pt->type == PTYPE_NUMBER) {
+        g_assert_cmpfloat(pt->value.number, ==, pt_copy->value.number);
+    } else if (pt->type == PTYPE_BOOLEAN) {
+        g_assert_cmpint(!!pt->value.max, ==, !!pt->value.max);
+    } else {
+        g_assert_cmpint(pt->value.max, ==, pt_copy->value.max);
+    }
+
+    ops->cleanup(serialize_data);
+    g_free(args);
+}
+
+static void test_struct(gconstpointer opaque)
+{
+    TestArgs *args = (TestArgs *) opaque;
+    const SerializeOps *ops = args->ops;
+    TestStruct *ts = struct_create();
+    TestStruct *ts_copy = NULL;
+    Error *err = NULL;
+    void *serialize_data;
+
+    ops->serialize(ts, &serialize_data, visit_struct, &err);
+    ops->deserialize((void **)&ts_copy, serialize_data, visit_struct, &err); 
+
+    g_assert(err == NULL);
+    struct_compare(ts, ts_copy);
+
+    struct_cleanup(ts);
+    struct_cleanup(ts_copy);
+
+    ops->cleanup(serialize_data);
+    g_free(args);
+}
+
+static void test_nested_struct(gconstpointer opaque)
+{
+    TestArgs *args = (TestArgs *) opaque;
+    const SerializeOps *ops = args->ops;
+    UserDefNested *udnp = nested_struct_create();
+    UserDefNested *udnp_copy = NULL;
+    Error *err = NULL;
+    void *serialize_data;
+    
+    ops->serialize(udnp, &serialize_data, visit_nested_struct, &err);
+    ops->deserialize((void **)&udnp_copy, serialize_data, visit_nested_struct, &err); 
+
+    g_assert(err == NULL);
+    nested_struct_compare(udnp, udnp_copy);
+
+    nested_struct_cleanup(udnp);
+    nested_struct_cleanup(udnp_copy);
+
+    ops->cleanup(serialize_data);
+    g_free(args);
+}
+
+static void test_nested_struct_list(gconstpointer opaque)
+{
+    TestArgs *args = (TestArgs *) opaque;
+    const SerializeOps *ops = args->ops;
+    UserDefNestedList *listp = NULL, *tmp, *tmp_copy, *listp_copy = NULL;
+    Error *err = NULL;
+    void *serialize_data;
+    int i = 0;
+
+    for (i = 0; i < 8; i++) {
+        tmp = g_malloc0(sizeof(UserDefNestedList));
+        tmp->value = nested_struct_create();
+        tmp->next = listp;
+        listp = tmp;
+    }
+    
+    ops->serialize(listp, &serialize_data, visit_nested_struct_list, &err);
+    ops->deserialize((void **)&listp_copy, serialize_data,
+                     visit_nested_struct_list, &err); 
+
+    g_assert(err == NULL);
+
+    tmp = listp;
+    tmp_copy = listp_copy;
+    while (listp_copy) {
+        g_assert(listp);
+        nested_struct_compare(listp->value, listp_copy->value);
+        listp = listp->next;
+        listp_copy = listp_copy->next;
+    }
+
+    qapi_free_UserDefNestedList(tmp);
+    qapi_free_UserDefNestedList(tmp_copy);
+
+    ops->cleanup(serialize_data);
+    g_free(args);
+}
+
+PrimitiveType pt_values[] = {
+    /* string tests */
+    {
+        .description = "string_empty",
+        .type = PTYPE_STRING,
+        .value.string = "",
+    },
+    {
+        .description = "string_whitespace",
+        .type = PTYPE_STRING,
+        .value.string = "a b  c\td",
+    },
+    {
+        .description = "string_newlines",
+        .type = PTYPE_STRING,
+        .value.string = "a\nb\n",
+    },
+    {
+        .description = "string_commas",
+        .type = PTYPE_STRING,
+        .value.string = "a,b, c,d",
+    },
+    {
+        .description = "string_single_quoted",
+        .type = PTYPE_STRING,
+        .value.string = "'a b',cd",
+    },
+    {
+        .description = "string_double_quoted",
+        .type = PTYPE_STRING,
+        .value.string = "\"a b\",cd",
+    },
+    /* boolean tests */
+    {
+        .description = "boolean_true1",
+        .type = PTYPE_BOOLEAN,
+        .value.boolean = true,
+    },
+    {
+        .description = "boolean_true2",
+        .type = PTYPE_BOOLEAN,
+        .value.boolean = 8,
+    },
+    {
+        .description = "boolean_true3",
+        .type = PTYPE_BOOLEAN,
+        .value.boolean = -1,
+    },
+    {
+        .description = "boolean_false1",
+        .type = PTYPE_BOOLEAN,
+        .value.boolean = false,
+    },
+    {
+        .description = "boolean_false2",
+        .type = PTYPE_BOOLEAN,
+        .value.boolean = 0,
+    },
+    /* number tests (double) */
+    {
+        .description = "number_sanity1",
+        .type = PTYPE_NUMBER,
+        .value.number = -1,
+    },
+    {
+        .description = "number_sanity2",
+        .type = PTYPE_NUMBER,
+        .value.number = 3.14159265,
+    },
+    {
+        .description = "number_min",
+        .type = PTYPE_NUMBER,
+        .value.number = DBL_MIN,
+    },
+    {
+        .description = "number_max",
+        .type = PTYPE_NUMBER,
+        .value.number = DBL_MAX,
+    },
+    /* integer tests (int64) */
+    {
+        .description = "integer_sanity1",
+        .type = PTYPE_INTEGER,
+        .value.integer = -1,
+    },
+    {
+        .description = "integer_sanity2",
+        .type = PTYPE_INTEGER,
+        .value.integer = INT64_MAX / 2 + 1,
+    },
+    {
+        .description = "integer_min",
+        .type = PTYPE_INTEGER,
+        .value.integer = INT64_MIN,
+    },
+    {
+        .description = "integer_max",
+        .type = PTYPE_INTEGER,
+        .value.integer = INT64_MAX,
+    },
+    /* uint8 tests */
+    {
+        .description = "uint8_sanity1",
+        .type = PTYPE_U8,
+        .value.u8 = 1,
+    },
+    {
+        .description = "uint8_sanity2",
+        .type = PTYPE_U8,
+        .value.u8 = UINT8_MAX / 2 + 1,
+    },
+    {
+        .description = "uint8_min",
+        .type = PTYPE_U8,
+        .value.u8 = 0,
+    },
+    {
+        .description = "uint8_max",
+        .type = PTYPE_U8,
+        .value.u8 = UINT8_MAX,
+    },
+    /* uint16 tests */
+    {
+        .description = "uint16_sanity1",
+        .type = PTYPE_U16,
+        .value.u16 = 1,
+    },
+    {
+        .description = "uint16_sanity2",
+        .type = PTYPE_U16,
+        .value.u16 = UINT16_MAX / 2 + 1,
+    },
+    {
+        .description = "uint16_min",
+        .type = PTYPE_U16,
+        .value.u16 = 0,
+    },
+    {
+        .description = "uint16_max",
+        .type = PTYPE_U16,
+        .value.u16 = UINT16_MAX,
+    },
+    /* uint32 tests */
+    {
+        .description = "uint32_sanity1",
+        .type = PTYPE_U32,
+        .value.u32 = 1,
+    },
+    {
+        .description = "uint32_sanity2",
+        .type = PTYPE_U32,
+        .value.u32 = UINT32_MAX / 2 + 1,
+    },
+    {
+        .description = "uint32_min",
+        .type = PTYPE_U32,
+        .value.u32 = 0,
+    },
+    {
+        .description = "uint32_max",
+        .type = PTYPE_U32,
+        .value.u32 = UINT32_MAX,
+    },
+    /* uint64 tests */
+    {
+        .description = "uint64_sanity1",
+        .type = PTYPE_U64,
+        .value.u64 = 1,
+    },
+    {
+        .description = "uint64_sanity2",
+        .type = PTYPE_U64,
+        .value.u64 = UINT64_MAX / 2 + 1,
+    },
+    {
+        .description = "uint64_min",
+        .type = PTYPE_U64,
+        .value.u64 = 0,
+    },
+    {
+        .description = "uint64_max",
+        .type = PTYPE_U64,
+        .value.u64 = UINT64_MAX,
+    },
+    /* int8 tests */
+    {
+        .description = "int8_sanity1",
+        .type = PTYPE_S8,
+        .value.s8 = -1,
+    },
+    {
+        .description = "int8_sanity2",
+        .type = PTYPE_S8,
+        .value.s8 = INT8_MAX / 2 + 1,
+    },
+    {
+        .description = "int8_min",
+        .type = PTYPE_S8,
+        .value.s8 = INT8_MIN,
+    },
+    {
+        .description = "int8_max",
+        .type = PTYPE_S8,
+        .value.s8 = INT8_MAX,
+    },
+    /* int16 tests */
+    {
+        .description = "int16_sanity1",
+        .type = PTYPE_S16,
+        .value.s16 = -1,
+    },
+    {
+        .description = "int16_sanity2",
+        .type = PTYPE_S16,
+        .value.s16 = INT16_MAX / 2 + 1,
+    },
+    {
+        .description = "int16_min",
+        .type = PTYPE_S16,
+        .value.s16 = INT16_MIN,
+    },
+    {
+        .description = "int16_max",
+        .type = PTYPE_S16,
+        .value.s16 = INT16_MAX,
+    },
+    /* int32 tests */
+    {
+        .description = "int32_sanity1",
+        .type = PTYPE_S32,
+        .value.s32 = -1,
+    },
+    {
+        .description = "int32_sanity2",
+        .type = PTYPE_S32,
+        .value.s32 = INT32_MAX / 2 + 1,
+    },
+    {
+        .description = "int32_min",
+        .type = PTYPE_S32,
+        .value.s32 = INT32_MIN,
+    },
+    {
+        .description = "int32_max",
+        .type = PTYPE_S32,
+        .value.s32 = INT32_MAX,
+    },
+    /* int64 tests */
+    {
+        .description = "int64_sanity1",
+        .type = PTYPE_S64,
+        .value.s64 = -1,
+    },
+    {
+        .description = "int64_sanity2",
+        .type = PTYPE_S64,
+        .value.s64 = INT64_MAX / 2 + 1,
+    },
+    {
+        .description = "int64_min",
+        .type = PTYPE_S64,
+        .value.s64 = INT64_MIN,
+    },
+    {
+        .description = "int64_max",
+        .type = PTYPE_S64,
+        .value.s64 = INT64_MAX,
+    },
+    { .type = PTYPE_EOL }
+};
+
+/* visitor-specific op implementations */
+
+typedef struct QmpSerializeData {
+    QmpOutputVisitor *qov;
+    QmpInputVisitor *qiv;
+} QmpSerializeData;
+
+static void qmp_serialize(void *native_in, void **datap,
+                          VisitorFunc visit, Error **errp)
+{
+    QmpSerializeData *d = g_malloc0(sizeof(*d));
+
+    d->qov = qmp_output_visitor_new();
+    visit(qmp_output_get_visitor(d->qov), &native_in, errp);
+    *datap = d;
+}
+
+static void qmp_deserialize(void **native_out, void *datap,
+                            VisitorFunc visit, Error **errp)
+{
+    QmpSerializeData *d = datap;
+
+    d->qiv = qmp_input_visitor_new(qmp_output_get_qobject(d->qov));
+    visit(qmp_input_get_visitor(d->qiv), native_out, errp);
+}
+
+static void qmp_cleanup(void *datap)
+{
+    QmpSerializeData *d = datap;
+    qmp_output_visitor_cleanup(d->qov);
+    qmp_input_visitor_cleanup(d->qiv);
+}
+
+/* visitor registration, test harness */
+
+/* note: to function interchangeably as a serialization mechanism your
+ * visitor test implementation should pass the test cases for all visitor
+ * capabilities: primitives, structures, and lists
+ */
+static const SerializeOps visitors[] = {
+    {
+        .type = "QMP",
+        .serialize = qmp_serialize,
+        .deserialize = qmp_deserialize,
+        .cleanup = qmp_cleanup,
+        .caps = VCAP_PRIMITIVES | VCAP_STRUCTURES | VCAP_LISTS
+    },
+    { NULL }
+};
+
+static void add_visitor_type(const SerializeOps *ops)
+{
+    char testname_prefix[128];
+    char testname[128];
+    TestArgs *args;
+    int i = 0;
+
+    sprintf(testname_prefix, "/visitor/serialization/%s", ops->type);
+
+    if (ops->caps & VCAP_PRIMITIVES) {
+        while (pt_values[i].type != PTYPE_EOL) {
+            sprintf(testname, "%s/primitives/%s", testname_prefix,
+                    pt_values[i].description);
+            args = g_malloc0(sizeof(*args));
+            args->ops = ops;
+            args->test_data = &pt_values[i];
+            g_test_add_data_func(testname, args, test_primitives);
+            i++;
+        }
+    }
+
+    if (ops->caps & VCAP_STRUCTURES) {
+        sprintf(testname, "%s/struct", testname_prefix);
+        args = g_malloc0(sizeof(*args));
+        args->ops = ops;
+        args->test_data = NULL;
+        g_test_add_data_func(testname, args, test_struct);
+
+        sprintf(testname, "%s/nested_struct", testname_prefix);
+        args = g_malloc0(sizeof(*args));
+        args->ops = ops;
+        args->test_data = NULL;
+        g_test_add_data_func(testname, args, test_nested_struct);
+    }
+
+    if (ops->caps & VCAP_LISTS) {
+        sprintf(testname, "%s/nested_struct_list", testname_prefix);
+        args = g_malloc0(sizeof(*args));
+        args->ops = ops;
+        args->test_data = NULL;
+        g_test_add_data_func(testname, args, test_nested_struct_list);
+    }
+}
+
+int main(int argc, char **argv)
+{
+    int i = 0;
+
+    g_test_init(&argc, &argv, NULL);
+
+    while (visitors[i].type != NULL) {
+        add_visitor_type(&visitors[i]);
+        i++;
+    }
+
+    g_test_run();
+
+    return 0;
+}
diff --git a/tests/Makefile b/tests/Makefile
index 74b29dc..55f3f04 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -1,6 +1,7 @@
 CHECKS = check-qdict check-qfloat check-qint check-qstring check-qlist
 CHECKS += check-qjson test-qmp-output-visitor test-qmp-input-visitor
 CHECKS += test-string-input-visitor test-string-output-visitor test-coroutine
+CHECKS += test-visitor-serialization
 
 check-qint.o check-qstring.o check-qdict.o check-qlist.o check-qfloat.o check-qjson.o test-coroutine.o: $(GENERATED_HEADERS)
 
@@ -14,7 +15,7 @@ test-coroutine: test-coroutine.o qemu-timer-common.o async.o $(coroutine-obj-y)
 
 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)
+	test-qmp-commands.o qemu-ga$(EXESUF) test-visitor-serialization: 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
@@ -42,6 +43,9 @@ test-qmp-input-visitor: test-qmp-input-visitor.o $(qobject-obj-y) $(qapi-obj-y)
 test-qmp-commands.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h test-qmp-marshal.c test-qmp-commands.h) $(qapi-obj-y)
 test-qmp-commands: test-qmp-commands.o $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o $(qapi-dir)/test-qmp-marshal.o module.o
 
+test-visitor-serialization.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h) $(qapi-obj-y)
+test-visitor-serialization$(EXESUF): test-visitor-serialization.o $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o
+
 .PHONY: check
 check: $(CHECKS)
 	$(call quiet-command, gtester $(CHECKS), "  CHECK")
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [Qemu-devel] [PATCH 3/3] qdev: switch property accessors to fixed-width visitor interfaces
  2012-02-22 19:00 [Qemu-devel] [PATCH 0/3] add fixed-width visitors and serialization tests Michael Roth
  2012-02-22 19:00 ` [Qemu-devel] [PATCH 1/3] qapi: add Visitor interfaces for uint*_t and int*_t Michael Roth
  2012-02-22 19:00 ` [Qemu-devel] [PATCH 2/3] qapi: unit tests for visitor-based serialization Michael Roth
@ 2012-02-22 19:00 ` Michael Roth
  2 siblings, 0 replies; 4+ messages in thread
From: Michael Roth @ 2012-02-22 19:00 UTC (permalink / raw)
  To: qemu-devel


Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 hw/qdev-addr.c       |    4 ++--
 hw/qdev-properties.c |   42 +++++++++++++++++-------------------------
 2 files changed, 19 insertions(+), 27 deletions(-)

diff --git a/hw/qdev-addr.c b/hw/qdev-addr.c
index 0bb16c7..b711b6b 100644
--- a/hw/qdev-addr.c
+++ b/hw/qdev-addr.c
@@ -27,7 +27,7 @@ static void get_taddr(Object *obj, Visitor *v, void *opaque,
     int64_t value;
 
     value = *ptr;
-    visit_type_int(v, &value, name, errp);
+    visit_type_int64(v, &value, name, errp);
 }
 
 static void set_taddr(Object *obj, Visitor *v, void *opaque,
@@ -44,7 +44,7 @@ static void set_taddr(Object *obj, Visitor *v, void *opaque,
         return;
     }
 
-    visit_type_int(v, &value, name, &local_err);
+    visit_type_int64(v, &value, name, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 0423af1..98d95fb 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -82,10 +82,8 @@ static void get_int8(Object *obj, Visitor *v, void *opaque,
     DeviceState *dev = DEVICE(obj);
     Property *prop = opaque;
     int8_t *ptr = qdev_get_prop_ptr(dev, prop);
-    int64_t value;
 
-    value = *ptr;
-    visit_type_int(v, &value, name, errp);
+    visit_type_int8(v, ptr, name, errp);
 }
 
 static void set_int8(Object *obj, Visitor *v, void *opaque,
@@ -93,16 +91,15 @@ static void set_int8(Object *obj, Visitor *v, void *opaque,
 {
     DeviceState *dev = DEVICE(obj);
     Property *prop = opaque;
-    int8_t *ptr = qdev_get_prop_ptr(dev, prop);
+    int8_t value, *ptr = qdev_get_prop_ptr(dev, prop);
     Error *local_err = NULL;
-    int64_t value;
 
     if (dev->state != DEV_STATE_CREATED) {
         error_set(errp, QERR_PERMISSION_DENIED);
         return;
     }
 
-    visit_type_int(v, &value, name, &local_err);
+    visit_type_int8(v, &value, name, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
@@ -111,7 +108,7 @@ static void set_int8(Object *obj, Visitor *v, void *opaque,
         *ptr = value;
     } else {
         error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
-                  dev->id?:"", name, value, prop->info->min,
+                  dev->id?:"", name, (int64_t)value, prop->info->min,
                   prop->info->max);
     }
 }
@@ -168,10 +165,8 @@ static void get_int16(Object *obj, Visitor *v, void *opaque,
     DeviceState *dev = DEVICE(obj);
     Property *prop = opaque;
     int16_t *ptr = qdev_get_prop_ptr(dev, prop);
-    int64_t value;
 
-    value = *ptr;
-    visit_type_int(v, &value, name, errp);
+    visit_type_int16(v, ptr, name, errp);
 }
 
 static void set_int16(Object *obj, Visitor *v, void *opaque,
@@ -179,16 +174,15 @@ static void set_int16(Object *obj, Visitor *v, void *opaque,
 {
     DeviceState *dev = DEVICE(obj);
     Property *prop = opaque;
-    int16_t *ptr = qdev_get_prop_ptr(dev, prop);
+    int16_t value, *ptr = qdev_get_prop_ptr(dev, prop);
     Error *local_err = NULL;
-    int64_t value;
 
     if (dev->state != DEV_STATE_CREATED) {
         error_set(errp, QERR_PERMISSION_DENIED);
         return;
     }
 
-    visit_type_int(v, &value, name, &local_err);
+    visit_type_int16(v, &value, name, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
@@ -197,7 +191,7 @@ static void set_int16(Object *obj, Visitor *v, void *opaque,
         *ptr = value;
     } else {
         error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
-                  dev->id?:"", name, value, prop->info->min,
+                  dev->id?:"", name, (int64_t)value, prop->info->min,
                   prop->info->max);
     }
 }
@@ -217,11 +211,10 @@ static void get_int32(Object *obj, Visitor *v, void *opaque,
 {
     DeviceState *dev = DEVICE(obj);
     Property *prop = opaque;
-    int32_t *ptr = qdev_get_prop_ptr(dev, prop);
-    int64_t value;
+    int32_t value, *ptr = qdev_get_prop_ptr(dev, prop);
 
     value = *ptr;
-    visit_type_int(v, &value, name, errp);
+    visit_type_int32(v, &value, name, errp);
 }
 
 static void set_int32(Object *obj, Visitor *v, void *opaque,
@@ -229,16 +222,15 @@ static void set_int32(Object *obj, Visitor *v, void *opaque,
 {
     DeviceState *dev = DEVICE(obj);
     Property *prop = opaque;
-    int32_t *ptr = qdev_get_prop_ptr(dev, prop);
+    int32_t value, *ptr = qdev_get_prop_ptr(dev, prop);
     Error *local_err = NULL;
-    int64_t value;
 
     if (dev->state != DEV_STATE_CREATED) {
         error_set(errp, QERR_PERMISSION_DENIED);
         return;
     }
 
-    visit_type_int(v, &value, name, &local_err);
+    visit_type_int32(v, &value, name, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
@@ -247,7 +239,7 @@ static void set_int32(Object *obj, Visitor *v, void *opaque,
         *ptr = value;
     } else {
         error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
-                  dev->id?:"", name, value, prop->info->min,
+                  dev->id?:"", name, (int64_t)value, prop->info->min,
                   prop->info->max);
     }
 }
@@ -313,7 +305,7 @@ static void get_int64(Object *obj, Visitor *v, void *opaque,
     Property *prop = opaque;
     int64_t *ptr = qdev_get_prop_ptr(dev, prop);
 
-    visit_type_int(v, ptr, name, errp);
+    visit_type_int64(v, ptr, name, errp);
 }
 
 static void set_int64(Object *obj, Visitor *v, void *opaque,
@@ -328,7 +320,7 @@ static void set_int64(Object *obj, Visitor *v, void *opaque,
         return;
     }
 
-    visit_type_int(v, ptr, name, errp);
+    visit_type_int64(v, ptr, name, errp);
 }
 
 PropertyInfo qdev_prop_uint64 = {
@@ -649,7 +641,7 @@ static void get_vlan(Object *obj, Visitor *v, void *opaque,
     int64_t id;
 
     id = *ptr ? (*ptr)->id : -1;
-    visit_type_int(v, &id, name, errp);
+    visit_type_int64(v, &id, name, errp);
 }
 
 static void set_vlan(Object *obj, Visitor *v, void *opaque,
@@ -667,7 +659,7 @@ static void set_vlan(Object *obj, Visitor *v, void *opaque,
         return;
     }
 
-    visit_type_int(v, &id, name, &local_err);
+    visit_type_int64(v, &id, name, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-02-22 19:01 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-22 19:00 [Qemu-devel] [PATCH 0/3] add fixed-width visitors and serialization tests Michael Roth
2012-02-22 19:00 ` [Qemu-devel] [PATCH 1/3] qapi: add Visitor interfaces for uint*_t and int*_t Michael Roth
2012-02-22 19:00 ` [Qemu-devel] [PATCH 2/3] qapi: unit tests for visitor-based serialization Michael Roth
2012-02-22 19:00 ` [Qemu-devel] [PATCH 3/3] qdev: switch property accessors to fixed-width visitor interfaces Michael Roth

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).