The functions simplify the handling of QOM properties whose type
is a QAPI struct. They go through a QObject just like the other
functions that access a QOM property through its C type.
Like QAPI_CLONE, the functions are wrapped by macros that take a
QAPI type name and use it to build the name of a visitor function.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qom/qom-qobject.h | 68 ++++++++++++
qom/qom-qobject.c | 49 +++++++++
tests/Makefile.include | 2 +-
tests/check-qom-proplist.c | 185 +++++++++++++++++++++++++++++++-
tests/qapi-schema/qapi-schema-test.json | 8 ++
5 files changed, 309 insertions(+), 3 deletions(-)
diff --git a/include/qom/qom-qobject.h b/include/qom/qom-qobject.h
index 77cd717..ff1d307 100644
--- a/include/qom/qom-qobject.h
+++ b/include/qom/qom-qobject.h
@@ -39,4 +39,72 @@ struct QObject *object_property_get_qobject(Object *obj, const char *name,
void object_property_set_qobject(Object *obj, struct QObject *qobj,
const char *name, struct Error **errp);
+/**
+ * object_property_get_ptr:
+ * @obj: the object
+ * @ptr: The C struct that will be written to the property.
+ * @name: the name of the property
+ * @visit_type: the visitor function for @ptr's type.
+ * @errp: returns an error if this function fails
+ *
+ * Return: the value of an object's property, unmarshaled into a C object
+ * through a QAPI type visitor, or NULL if an error occurs.
+ */
+void *object_property_get_ptr(Object *obj, const char *name,
+ void (*visit_type)(Visitor *, const char *,
+ void **, Error **),
+ Error **errp);
+
+/**
+ * OBJECT_PROPERTY_GET_PTR:
+ * @obj: the object
+ * @ptr: The C struct that will be written to the property.
+ * @name: the name of the property
+ * @type: the name of the C struct type pointed to by @ptr.
+ * @errp: returns an error if this function fails
+ *
+ * Return: the value of an object's property, unmarshaled into a C object
+ * through a QAPI type visitor, or NULL if an error occurs.
+ */
+#define OBJECT_PROPERTY_GET_PTR(obj, name, type, errp) \
+ ((type *) \
+ object_property_get_ptr(obj, name, \
+ (void (*)(Visitor *, const char *, void**, \
+ Error **))visit_type_ ## type, \
+ errp))
+
+/**
+ * object_property_set_ptr:
+ * @obj: the object
+ * @ptr: The C struct that will be written to the property.
+ * @name: the name of the property
+ * @visit_type: the visitor function for @ptr's type.
+ * @errp: returns an error if this function fails
+ *
+ * Sets an object's property to a C object's value, using a QAPI
+ * type visitor to marshal the C struct into the object.
+ */
+void object_property_set_ptr(Object *obj, void *ptr, const char *name,
+ void (*visit_type)(Visitor *, const char *,
+ void **, Error **),
+ Error **errp);
+
+/**
+ * OBJECT_PROPERTY_SET_PTR:
+ * @obj: the object
+ * @ptr: The C struct that will be written to the property.
+ * @name: the name of the property
+ * @type: the name of the C struct type pointed to by @ptr.
+ * @errp: returns an error if this function fails
+ *
+ * Sets an object's property to a C object's value, using a QAPI
+ * type visitor to marshal the C struct into the object.
+ */
+#define OBJECT_PROPERTY_SET_PTR(obj, ptr, name, type, errp) \
+ object_property_set_ptr(obj, ptr + type_check(type, typeof(*ptr)), \
+ name, \
+ (void (*)(Visitor *, const char *, void**, \
+ Error **))visit_type_ ## type, \
+ errp)
+
#endif
diff --git a/qom/qom-qobject.c b/qom/qom-qobject.c
index 447e4a0..09a12e0 100644
--- a/qom/qom-qobject.c
+++ b/qom/qom-qobject.c
@@ -44,3 +44,52 @@ QObject *object_property_get_qobject(Object *obj, const char *name,
visit_free(v);
return ret;
}
+
+void object_property_set_ptr(Object *obj, void *ptr, const char *name,
+ void (*visit_type)(Visitor *, const char *, void **, Error **),
+ Error **errp)
+{
+ Error *local_err = NULL;
+ QObject *ret = NULL;
+ Visitor *v;
+ v = qobject_output_visitor_new(&ret);
+ visit_type(v, name, &ptr, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ visit_free(v);
+ return;
+ }
+ visit_complete(v, &ret);
+ visit_free(v);
+
+ /* Do not use object_property_set_qobject until we switch it
+ * to use qobject_input_visitor_new in strict mode. See the
+ * /qom/proplist/get-set-ptr/contravariant unit test.
+ */
+ v = qobject_input_visitor_new(ret, true);
+ object_property_set(obj, v, name, errp);
+ visit_free(v);
+ qobject_decref(ret);
+}
+
+void *object_property_get_ptr(Object *obj, const char *name,
+ void (*visit_type)(Visitor *, const char *, void **, Error **),
+ Error **errp)
+{
+ QObject *ret;
+ Visitor *v;
+ void *ptr = NULL;
+
+ ret = object_property_get_qobject(obj, name, errp);
+ if (!ret) {
+ return NULL;
+ }
+
+ /* Do not enable strict mode to allow passing covariant
+ * data types.
+ */
+ v = qobject_input_visitor_new(ret, false);
+ visit_type(v, name, &ptr, errp);
+ qobject_decref(ret);
+ return ptr;
+}
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 634394a..9de910b 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -515,7 +515,7 @@ tests/check-qfloat$(EXESUF): tests/check-qfloat.o $(test-util-obj-y)
tests/check-qnull$(EXESUF): tests/check-qnull.o $(test-util-obj-y)
tests/check-qjson$(EXESUF): tests/check-qjson.o $(test-util-obj-y)
tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o $(test-qom-obj-y)
-tests/check-qom-proplist$(EXESUF): tests/check-qom-proplist.o $(test-qom-obj-y)
+tests/check-qom-proplist$(EXESUF): tests/check-qom-proplist.o $(test-qom-obj-y) $(test-qapi-obj-y)
tests/test-char$(EXESUF): tests/test-char.o qemu-timer.o \
$(test-util-obj-y) $(qtest-obj-y) $(test-block-obj-y) $(chardev-obj-y)
diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
index a16cefc..e0ad880 100644
--- a/tests/check-qom-proplist.c
+++ b/tests/check-qom-proplist.c
@@ -22,8 +22,11 @@
#include "qapi/error.h"
#include "qom/object.h"
+#include "qom/qom-qobject.h"
#include "qemu/module.h"
+#include "test-qapi-types.h"
+#include "test-qapi-visit.h"
#define TYPE_DUMMY "qemu-dummy"
@@ -56,6 +59,8 @@ struct DummyObject {
bool bv;
DummyAnimal av;
char *sv;
+
+ UserDefOne *qv;
};
struct DummyObjectClass {
@@ -120,12 +125,42 @@ static char *dummy_get_sv(Object *obj,
static void dummy_init(Object *obj)
{
+ DummyObject *dobj = DUMMY_OBJECT(obj);
+
object_property_add_bool(obj, "bv",
dummy_get_bv,
dummy_set_bv,
NULL);
+ dobj->qv = g_new0(UserDefOne, 1);
+ dobj->qv->string = g_strdup("dummy string");
+}
+
+
+static void dummy_get_qv(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ DummyObject *dobj = DUMMY_OBJECT(obj);
+
+ visit_type_UserDefOne(v, name, &dobj->qv, errp);
}
+static void dummy_set_qv(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ DummyObject *dobj = DUMMY_OBJECT(obj);
+ UserDefOne *qv = NULL;
+ Error *local_err = NULL;
+
+ visit_type_UserDefOne(v, name, &qv, &local_err);
+ if (local_err) {
+ g_assert(qv == NULL);
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ qapi_free_UserDefOne(dobj->qv);
+ dobj->qv = qv;
+}
static void dummy_class_init(ObjectClass *cls, void *data)
{
@@ -143,6 +178,13 @@ static void dummy_class_init(ObjectClass *cls, void *data)
dummy_get_av,
dummy_set_av,
NULL);
+ object_class_property_add(cls, "qv",
+ "UserDefOne",
+ dummy_get_qv,
+ dummy_set_qv,
+ NULL,
+ NULL,
+ NULL);
}
@@ -151,9 +193,9 @@ static void dummy_finalize(Object *obj)
DummyObject *dobj = DUMMY_OBJECT(obj);
g_free(dobj->sv);
+ qapi_free_UserDefOne(dobj->qv);
}
-
static const TypeInfo dummy_info = {
.name = TYPE_DUMMY,
.parent = TYPE_OBJECT,
@@ -473,7 +515,8 @@ static void test_dummy_iterator(void)
ObjectProperty *prop;
ObjectPropertyIterator iter;
- bool seenbv = false, seensv = false, seenav = false, seentype;
+ bool seenbv = false, seensv = false, seenav = false;
+ bool seenqv = false, seentype = false;
object_property_iter_init(&iter, OBJECT(dobj));
while ((prop = object_property_iter_next(&iter))) {
@@ -483,6 +526,8 @@ static void test_dummy_iterator(void)
seensv = true;
} else if (g_str_equal(prop->name, "av")) {
seenav = true;
+ } else if (g_str_equal(prop->name, "qv")) {
+ seenqv = true;
} else if (g_str_equal(prop->name, "type")) {
/* This prop comes from the base Object class */
seentype = true;
@@ -494,6 +539,7 @@ static void test_dummy_iterator(void)
g_assert(seenbv);
g_assert(seenav);
g_assert(seensv);
+ g_assert(seenqv);
g_assert(seentype);
object_unparent(OBJECT(dobj));
@@ -513,6 +559,137 @@ static void test_dummy_delchild(void)
object_unparent(OBJECT(dev));
}
+static void test_dummy_get_set_ptr_struct(void)
+{
+ DummyObject *dobj = DUMMY_OBJECT(object_new(TYPE_DUMMY));
+ Error *local_err = NULL;
+ const char *s = "my other dummy string";
+ UserDefOne *ret;
+ UserDefOne val;
+
+ ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "qv",
+ UserDefOne, &local_err);
+ g_assert(!local_err);
+
+ g_assert_cmpint(ret->integer, ==, 0);
+ g_assert_cmpstr(ret->string, ==, "dummy string");
+ g_assert(!ret->has_enum1);
+ qapi_free_UserDefOne(ret);
+
+ val.integer = 42;
+ val.string = g_strdup(s);
+ val.has_enum1 = true;
+ val.enum1 = ENUM_ONE_VALUE1;
+ OBJECT_PROPERTY_SET_PTR(OBJECT(dobj), &val, "qv",
+ UserDefOne, &local_err);
+ g_assert(!local_err);
+
+ ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "qv",
+ UserDefOne, &local_err);
+ g_assert(!local_err);
+
+ g_assert_cmpint(ret->integer, ==, val.integer);
+ g_assert_cmpstr(ret->string, ==, val.string);
+ g_assert(ret->has_enum1);
+ g_assert_cmpint(ret->enum1, ==, val.enum1);
+ g_free(val.string);
+ qapi_free_UserDefOne(ret);
+}
+
+static void test_dummy_get_set_ptr_contravariant(void)
+{
+ DummyObject *dobj = DUMMY_OBJECT(object_new(TYPE_DUMMY));
+ Error *local_err = NULL;
+ UserDefOneMore *ret;
+ UserDefOneMore val;
+
+ /* You cannot retrieve a contravariant (subclass) type... */
+ ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "qv",
+ UserDefOneMore, &local_err);
+ g_assert(local_err);
+ g_assert(!ret);
+ error_free(local_err);
+ local_err = NULL;
+
+ /* And you cannot set one either. */
+ val.integer = 42;
+ val.string = g_strdup("unused");
+ val.has_enum1 = false;
+ val.boolean = false;
+
+ OBJECT_PROPERTY_SET_PTR(OBJECT(dobj), &val, "qv",
+ UserDefOneMore, &local_err);
+ g_assert(local_err);
+}
+
+static void test_dummy_get_set_ptr_covariant(void)
+{
+ DummyObject *dobj = DUMMY_OBJECT(object_new(TYPE_DUMMY));
+ Error *local_err = NULL;
+ UserDefZero *ret;
+ UserDefZero val;
+
+ /* You can retrieve a covariant (superclass) type... */
+ ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "qv",
+ UserDefZero, &local_err);
+ g_assert(!local_err);
+
+ g_assert_cmpint(ret->integer, ==, 0);
+ qapi_free_UserDefZero(ret);
+
+ /* But you cannot set one. */
+ val.integer = 42;
+ OBJECT_PROPERTY_SET_PTR(OBJECT(dobj), &val, "qv",
+ UserDefZero, &local_err);
+ g_assert(local_err);
+ error_free(local_err);
+ local_err = NULL;
+
+ /* Test that the property has not been modified at all */
+ ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "qv",
+ UserDefZero, &local_err);
+ g_assert(!local_err);
+
+ g_assert_cmpint(ret->integer, ==, 0);
+ qapi_free_UserDefZero(ret);
+}
+
+static void test_dummy_get_set_ptr_error(void)
+{
+ DummyObject *dobj = DUMMY_OBJECT(object_new(TYPE_DUMMY));
+ Error *local_err = NULL;
+ const char *s = "my other dummy string";
+ UserDefOne *ret;
+ UserDefOne val;
+
+ ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "blah",
+ UserDefOne, &local_err);
+ g_assert(local_err);
+ g_assert(!ret);
+ error_free(local_err);
+ local_err = NULL;
+
+ val.integer = 42;
+ val.string = g_strdup(s);
+ val.has_enum1 = true;
+ val.enum1 = 100;
+ OBJECT_PROPERTY_SET_PTR(OBJECT(dobj), &val, "qv",
+ UserDefOne, &local_err);
+ g_assert(local_err);
+ error_free(local_err);
+ local_err = NULL;
+
+ ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "qv",
+ UserDefOne, &local_err);
+ g_assert(!local_err);
+
+ /* Test that the property has not been modified at all */
+ g_assert_cmpint(ret->integer, ==, 0);
+ g_assert_cmpstr(ret->string, ==, "dummy string");
+ g_assert(!ret->has_enum1);
+ qapi_free_UserDefOne(ret);
+}
+
int main(int argc, char **argv)
{
g_test_init(&argc, &argv, NULL);
@@ -530,5 +707,9 @@ int main(int argc, char **argv)
g_test_add_func("/qom/proplist/iterator", test_dummy_iterator);
g_test_add_func("/qom/proplist/delchild", test_dummy_delchild);
+ g_test_add_func("/qom/proplist/get-set-ptr/struct", test_dummy_get_set_ptr_struct);
+ g_test_add_func("/qom/proplist/get-set-ptr/error", test_dummy_get_set_ptr_error);
+ g_test_add_func("/qom/proplist/get-set-ptr/covariant", test_dummy_get_set_ptr_covariant);
+ g_test_add_func("/qom/proplist/get-set-ptr/contravariant", test_dummy_get_set_ptr_contravariant);
return g_test_run();
}
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index f4d8cc4..4e3f6ff 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -91,6 +91,14 @@
'*enum1': 'EnumOne' } } # intentional forward reference
##
+# @UserDefOneMore:
+# for testing nested structs
+##
+{ 'struct': 'UserDefOneMore',
+ 'base': 'UserDefOne',
+ 'data': { 'boolean': 'bool' } }
+
+##
# @EnumOne:
##
{ 'enum': 'EnumOne',
--
2.9.3
On 21/02/2017 11:42, Paolo Bonzini wrote:
> The functions simplify the handling of QOM properties whose type
> is a QAPI struct. They go through a QObject just like the other
> functions that access a QOM property through its C type.
>
> Like QAPI_CLONE, the functions are wrapped by macros that take a
> QAPI type name and use it to build the name of a visitor function.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> include/qom/qom-qobject.h | 68 ++++++++++++
> qom/qom-qobject.c | 49 +++++++++
> tests/Makefile.include | 2 +-
> tests/check-qom-proplist.c | 185 +++++++++++++++++++++++++++++++-
> tests/qapi-schema/qapi-schema-test.json | 8 ++
> 5 files changed, 309 insertions(+), 3 deletions(-)
Missing hunk:
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index bc8d496..d3a2990 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -107,6 +107,9 @@ object UserDefOne
base UserDefZero
member string: str optional=False
member enum1: EnumOne optional=True
+object UserDefOneMore
+ base UserDefOne
+ member boolean: bool optional=False
object UserDefOptions
member i64: intList optional=True
member u64: uint64List optional=True
@@ -283,6 +286,9 @@ for testing override of default naming heuristic
doc symbol=UserDefOne expr=('struct', 'UserDefOne')
body=
for testing nested structs
+doc symbol=UserDefOneMore expr=('struct', 'UserDefOneMore')
+ body=
+for testing nested structs
doc symbol=EnumOne expr=('enum', 'EnumOne')
doc symbol=UserDefZero expr=('struct', 'UserDefZero')
doc symbol=UserDefTwoDictDict expr=('struct', 'UserDefTwoDictDict')
Paolo
> diff --git a/include/qom/qom-qobject.h b/include/qom/qom-qobject.h
> index 77cd717..ff1d307 100644
> --- a/include/qom/qom-qobject.h
> +++ b/include/qom/qom-qobject.h
> @@ -39,4 +39,72 @@ struct QObject *object_property_get_qobject(Object *obj, const char *name,
> void object_property_set_qobject(Object *obj, struct QObject *qobj,
> const char *name, struct Error **errp);
>
> +/**
> + * object_property_get_ptr:
> + * @obj: the object
> + * @ptr: The C struct that will be written to the property.
> + * @name: the name of the property
> + * @visit_type: the visitor function for @ptr's type.
> + * @errp: returns an error if this function fails
> + *
> + * Return: the value of an object's property, unmarshaled into a C object
> + * through a QAPI type visitor, or NULL if an error occurs.
> + */
> +void *object_property_get_ptr(Object *obj, const char *name,
> + void (*visit_type)(Visitor *, const char *,
> + void **, Error **),
> + Error **errp);
> +
> +/**
> + * OBJECT_PROPERTY_GET_PTR:
> + * @obj: the object
> + * @ptr: The C struct that will be written to the property.
> + * @name: the name of the property
> + * @type: the name of the C struct type pointed to by @ptr.
> + * @errp: returns an error if this function fails
> + *
> + * Return: the value of an object's property, unmarshaled into a C object
> + * through a QAPI type visitor, or NULL if an error occurs.
> + */
> +#define OBJECT_PROPERTY_GET_PTR(obj, name, type, errp) \
> + ((type *) \
> + object_property_get_ptr(obj, name, \
> + (void (*)(Visitor *, const char *, void**, \
> + Error **))visit_type_ ## type, \
> + errp))
> +
> +/**
> + * object_property_set_ptr:
> + * @obj: the object
> + * @ptr: The C struct that will be written to the property.
> + * @name: the name of the property
> + * @visit_type: the visitor function for @ptr's type.
> + * @errp: returns an error if this function fails
> + *
> + * Sets an object's property to a C object's value, using a QAPI
> + * type visitor to marshal the C struct into the object.
> + */
> +void object_property_set_ptr(Object *obj, void *ptr, const char *name,
> + void (*visit_type)(Visitor *, const char *,
> + void **, Error **),
> + Error **errp);
> +
> +/**
> + * OBJECT_PROPERTY_SET_PTR:
> + * @obj: the object
> + * @ptr: The C struct that will be written to the property.
> + * @name: the name of the property
> + * @type: the name of the C struct type pointed to by @ptr.
> + * @errp: returns an error if this function fails
> + *
> + * Sets an object's property to a C object's value, using a QAPI
> + * type visitor to marshal the C struct into the object.
> + */
> +#define OBJECT_PROPERTY_SET_PTR(obj, ptr, name, type, errp) \
> + object_property_set_ptr(obj, ptr + type_check(type, typeof(*ptr)), \
> + name, \
> + (void (*)(Visitor *, const char *, void**, \
> + Error **))visit_type_ ## type, \
> + errp)
> +
> #endif
> diff --git a/qom/qom-qobject.c b/qom/qom-qobject.c
> index 447e4a0..09a12e0 100644
> --- a/qom/qom-qobject.c
> +++ b/qom/qom-qobject.c
> @@ -44,3 +44,52 @@ QObject *object_property_get_qobject(Object *obj, const char *name,
> visit_free(v);
> return ret;
> }
> +
> +void object_property_set_ptr(Object *obj, void *ptr, const char *name,
> + void (*visit_type)(Visitor *, const char *, void **, Error **),
> + Error **errp)
> +{
> + Error *local_err = NULL;
> + QObject *ret = NULL;
> + Visitor *v;
> + v = qobject_output_visitor_new(&ret);
> + visit_type(v, name, &ptr, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + visit_free(v);
> + return;
> + }
> + visit_complete(v, &ret);
> + visit_free(v);
> +
> + /* Do not use object_property_set_qobject until we switch it
> + * to use qobject_input_visitor_new in strict mode. See the
> + * /qom/proplist/get-set-ptr/contravariant unit test.
> + */
> + v = qobject_input_visitor_new(ret, true);
> + object_property_set(obj, v, name, errp);
> + visit_free(v);
> + qobject_decref(ret);
> +}
> +
> +void *object_property_get_ptr(Object *obj, const char *name,
> + void (*visit_type)(Visitor *, const char *, void **, Error **),
> + Error **errp)
> +{
> + QObject *ret;
> + Visitor *v;
> + void *ptr = NULL;
> +
> + ret = object_property_get_qobject(obj, name, errp);
> + if (!ret) {
> + return NULL;
> + }
> +
> + /* Do not enable strict mode to allow passing covariant
> + * data types.
> + */
> + v = qobject_input_visitor_new(ret, false);
> + visit_type(v, name, &ptr, errp);
> + qobject_decref(ret);
> + return ptr;
> +}
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 634394a..9de910b 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -515,7 +515,7 @@ tests/check-qfloat$(EXESUF): tests/check-qfloat.o $(test-util-obj-y)
> tests/check-qnull$(EXESUF): tests/check-qnull.o $(test-util-obj-y)
> tests/check-qjson$(EXESUF): tests/check-qjson.o $(test-util-obj-y)
> tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o $(test-qom-obj-y)
> -tests/check-qom-proplist$(EXESUF): tests/check-qom-proplist.o $(test-qom-obj-y)
> +tests/check-qom-proplist$(EXESUF): tests/check-qom-proplist.o $(test-qom-obj-y) $(test-qapi-obj-y)
>
> tests/test-char$(EXESUF): tests/test-char.o qemu-timer.o \
> $(test-util-obj-y) $(qtest-obj-y) $(test-block-obj-y) $(chardev-obj-y)
> diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
> index a16cefc..e0ad880 100644
> --- a/tests/check-qom-proplist.c
> +++ b/tests/check-qom-proplist.c
> @@ -22,8 +22,11 @@
>
> #include "qapi/error.h"
> #include "qom/object.h"
> +#include "qom/qom-qobject.h"
> #include "qemu/module.h"
>
> +#include "test-qapi-types.h"
> +#include "test-qapi-visit.h"
>
> #define TYPE_DUMMY "qemu-dummy"
>
> @@ -56,6 +59,8 @@ struct DummyObject {
> bool bv;
> DummyAnimal av;
> char *sv;
> +
> + UserDefOne *qv;
> };
>
> struct DummyObjectClass {
> @@ -120,12 +125,42 @@ static char *dummy_get_sv(Object *obj,
>
> static void dummy_init(Object *obj)
> {
> + DummyObject *dobj = DUMMY_OBJECT(obj);
> +
> object_property_add_bool(obj, "bv",
> dummy_get_bv,
> dummy_set_bv,
> NULL);
> + dobj->qv = g_new0(UserDefOne, 1);
> + dobj->qv->string = g_strdup("dummy string");
> +}
> +
> +
> +static void dummy_get_qv(Object *obj, Visitor *v, const char *name,
> + void *opaque, Error **errp)
> +{
> + DummyObject *dobj = DUMMY_OBJECT(obj);
> +
> + visit_type_UserDefOne(v, name, &dobj->qv, errp);
> }
>
> +static void dummy_set_qv(Object *obj, Visitor *v, const char *name,
> + void *opaque, Error **errp)
> +{
> + DummyObject *dobj = DUMMY_OBJECT(obj);
> + UserDefOne *qv = NULL;
> + Error *local_err = NULL;
> +
> + visit_type_UserDefOne(v, name, &qv, &local_err);
> + if (local_err) {
> + g_assert(qv == NULL);
> + error_propagate(errp, local_err);
> + return;
> + }
> +
> + qapi_free_UserDefOne(dobj->qv);
> + dobj->qv = qv;
> +}
>
> static void dummy_class_init(ObjectClass *cls, void *data)
> {
> @@ -143,6 +178,13 @@ static void dummy_class_init(ObjectClass *cls, void *data)
> dummy_get_av,
> dummy_set_av,
> NULL);
> + object_class_property_add(cls, "qv",
> + "UserDefOne",
> + dummy_get_qv,
> + dummy_set_qv,
> + NULL,
> + NULL,
> + NULL);
> }
>
>
> @@ -151,9 +193,9 @@ static void dummy_finalize(Object *obj)
> DummyObject *dobj = DUMMY_OBJECT(obj);
>
> g_free(dobj->sv);
> + qapi_free_UserDefOne(dobj->qv);
> }
>
> -
> static const TypeInfo dummy_info = {
> .name = TYPE_DUMMY,
> .parent = TYPE_OBJECT,
> @@ -473,7 +515,8 @@ static void test_dummy_iterator(void)
>
> ObjectProperty *prop;
> ObjectPropertyIterator iter;
> - bool seenbv = false, seensv = false, seenav = false, seentype;
> + bool seenbv = false, seensv = false, seenav = false;
> + bool seenqv = false, seentype = false;
>
> object_property_iter_init(&iter, OBJECT(dobj));
> while ((prop = object_property_iter_next(&iter))) {
> @@ -483,6 +526,8 @@ static void test_dummy_iterator(void)
> seensv = true;
> } else if (g_str_equal(prop->name, "av")) {
> seenav = true;
> + } else if (g_str_equal(prop->name, "qv")) {
> + seenqv = true;
> } else if (g_str_equal(prop->name, "type")) {
> /* This prop comes from the base Object class */
> seentype = true;
> @@ -494,6 +539,7 @@ static void test_dummy_iterator(void)
> g_assert(seenbv);
> g_assert(seenav);
> g_assert(seensv);
> + g_assert(seenqv);
> g_assert(seentype);
>
> object_unparent(OBJECT(dobj));
> @@ -513,6 +559,137 @@ static void test_dummy_delchild(void)
> object_unparent(OBJECT(dev));
> }
>
> +static void test_dummy_get_set_ptr_struct(void)
> +{
> + DummyObject *dobj = DUMMY_OBJECT(object_new(TYPE_DUMMY));
> + Error *local_err = NULL;
> + const char *s = "my other dummy string";
> + UserDefOne *ret;
> + UserDefOne val;
> +
> + ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "qv",
> + UserDefOne, &local_err);
> + g_assert(!local_err);
> +
> + g_assert_cmpint(ret->integer, ==, 0);
> + g_assert_cmpstr(ret->string, ==, "dummy string");
> + g_assert(!ret->has_enum1);
> + qapi_free_UserDefOne(ret);
> +
> + val.integer = 42;
> + val.string = g_strdup(s);
> + val.has_enum1 = true;
> + val.enum1 = ENUM_ONE_VALUE1;
> + OBJECT_PROPERTY_SET_PTR(OBJECT(dobj), &val, "qv",
> + UserDefOne, &local_err);
> + g_assert(!local_err);
> +
> + ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "qv",
> + UserDefOne, &local_err);
> + g_assert(!local_err);
> +
> + g_assert_cmpint(ret->integer, ==, val.integer);
> + g_assert_cmpstr(ret->string, ==, val.string);
> + g_assert(ret->has_enum1);
> + g_assert_cmpint(ret->enum1, ==, val.enum1);
> + g_free(val.string);
> + qapi_free_UserDefOne(ret);
> +}
> +
> +static void test_dummy_get_set_ptr_contravariant(void)
> +{
> + DummyObject *dobj = DUMMY_OBJECT(object_new(TYPE_DUMMY));
> + Error *local_err = NULL;
> + UserDefOneMore *ret;
> + UserDefOneMore val;
> +
> + /* You cannot retrieve a contravariant (subclass) type... */
> + ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "qv",
> + UserDefOneMore, &local_err);
> + g_assert(local_err);
> + g_assert(!ret);
> + error_free(local_err);
> + local_err = NULL;
> +
> + /* And you cannot set one either. */
> + val.integer = 42;
> + val.string = g_strdup("unused");
> + val.has_enum1 = false;
> + val.boolean = false;
> +
> + OBJECT_PROPERTY_SET_PTR(OBJECT(dobj), &val, "qv",
> + UserDefOneMore, &local_err);
> + g_assert(local_err);
> +}
> +
> +static void test_dummy_get_set_ptr_covariant(void)
> +{
> + DummyObject *dobj = DUMMY_OBJECT(object_new(TYPE_DUMMY));
> + Error *local_err = NULL;
> + UserDefZero *ret;
> + UserDefZero val;
> +
> + /* You can retrieve a covariant (superclass) type... */
> + ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "qv",
> + UserDefZero, &local_err);
> + g_assert(!local_err);
> +
> + g_assert_cmpint(ret->integer, ==, 0);
> + qapi_free_UserDefZero(ret);
> +
> + /* But you cannot set one. */
> + val.integer = 42;
> + OBJECT_PROPERTY_SET_PTR(OBJECT(dobj), &val, "qv",
> + UserDefZero, &local_err);
> + g_assert(local_err);
> + error_free(local_err);
> + local_err = NULL;
> +
> + /* Test that the property has not been modified at all */
> + ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "qv",
> + UserDefZero, &local_err);
> + g_assert(!local_err);
> +
> + g_assert_cmpint(ret->integer, ==, 0);
> + qapi_free_UserDefZero(ret);
> +}
> +
> +static void test_dummy_get_set_ptr_error(void)
> +{
> + DummyObject *dobj = DUMMY_OBJECT(object_new(TYPE_DUMMY));
> + Error *local_err = NULL;
> + const char *s = "my other dummy string";
> + UserDefOne *ret;
> + UserDefOne val;
> +
> + ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "blah",
> + UserDefOne, &local_err);
> + g_assert(local_err);
> + g_assert(!ret);
> + error_free(local_err);
> + local_err = NULL;
> +
> + val.integer = 42;
> + val.string = g_strdup(s);
> + val.has_enum1 = true;
> + val.enum1 = 100;
> + OBJECT_PROPERTY_SET_PTR(OBJECT(dobj), &val, "qv",
> + UserDefOne, &local_err);
> + g_assert(local_err);
> + error_free(local_err);
> + local_err = NULL;
> +
> + ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "qv",
> + UserDefOne, &local_err);
> + g_assert(!local_err);
> +
> + /* Test that the property has not been modified at all */
> + g_assert_cmpint(ret->integer, ==, 0);
> + g_assert_cmpstr(ret->string, ==, "dummy string");
> + g_assert(!ret->has_enum1);
> + qapi_free_UserDefOne(ret);
> +}
> +
> int main(int argc, char **argv)
> {
> g_test_init(&argc, &argv, NULL);
> @@ -530,5 +707,9 @@ int main(int argc, char **argv)
> g_test_add_func("/qom/proplist/iterator", test_dummy_iterator);
> g_test_add_func("/qom/proplist/delchild", test_dummy_delchild);
>
> + g_test_add_func("/qom/proplist/get-set-ptr/struct", test_dummy_get_set_ptr_struct);
> + g_test_add_func("/qom/proplist/get-set-ptr/error", test_dummy_get_set_ptr_error);
> + g_test_add_func("/qom/proplist/get-set-ptr/covariant", test_dummy_get_set_ptr_covariant);
> + g_test_add_func("/qom/proplist/get-set-ptr/contravariant", test_dummy_get_set_ptr_contravariant);
> return g_test_run();
> }
> diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
> index f4d8cc4..4e3f6ff 100644
> --- a/tests/qapi-schema/qapi-schema-test.json
> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -91,6 +91,14 @@
> '*enum1': 'EnumOne' } } # intentional forward reference
>
> ##
> +# @UserDefOneMore:
> +# for testing nested structs
> +##
> +{ 'struct': 'UserDefOneMore',
> + 'base': 'UserDefOne',
> + 'data': { 'boolean': 'bool' } }
> +
> +##
> # @EnumOne:
> ##
> { 'enum': 'EnumOne',
>
Hi
On Tue, Feb 21, 2017 at 4:17 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 21/02/2017 11:42, Paolo Bonzini wrote:
> > The functions simplify the handling of QOM properties whose type
> > is a QAPI struct. They go through a QObject just like the other
> > functions that access a QOM property through its C type.
> >
> > Like QAPI_CLONE, the functions are wrapped by macros that take a
> > QAPI type name and use it to build the name of a visitor function.
> >
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> > include/qom/qom-qobject.h | 68 ++++++++++++
> > qom/qom-qobject.c | 49 +++++++++
> > tests/Makefile.include | 2 +-
> > tests/check-qom-proplist.c | 185
> +++++++++++++++++++++++++++++++-
> > tests/qapi-schema/qapi-schema-test.json | 8 ++
> > 5 files changed, 309 insertions(+), 3 deletions(-)
>
> Missing hunk:
>
> diff --git a/tests/qapi-schema/qapi-schema-test.out
> b/tests/qapi-schema/qapi-schema-test.out
> index bc8d496..d3a2990 100644
> --- a/tests/qapi-schema/qapi-schema-test.out
> +++ b/tests/qapi-schema/qapi-schema-test.out
> @@ -107,6 +107,9 @@ object UserDefOne
> base UserDefZero
> member string: str optional=False
> member enum1: EnumOne optional=True
> +object UserDefOneMore
> + base UserDefOne
> + member boolean: bool optional=False
> object UserDefOptions
> member i64: intList optional=True
> member u64: uint64List optional=True
> @@ -283,6 +286,9 @@ for testing override of default naming heuristic
> doc symbol=UserDefOne expr=('struct', 'UserDefOne')
> body=
> for testing nested structs
> +doc symbol=UserDefOneMore expr=('struct', 'UserDefOneMore')
> + body=
> +for testing nested structs
> doc symbol=EnumOne expr=('enum', 'EnumOne')
> doc symbol=UserDefZero expr=('struct', 'UserDefZero')
> doc symbol=UserDefTwoDictDict expr=('struct', 'UserDefTwoDictDict')
>
> Paolo
>
> > diff --git a/include/qom/qom-qobject.h b/include/qom/qom-qobject.h
> > index 77cd717..ff1d307 100644
> > --- a/include/qom/qom-qobject.h
> > +++ b/include/qom/qom-qobject.h
> > @@ -39,4 +39,72 @@ struct QObject *object_property_get_qobject(Object
> *obj, const char *name,
> > void object_property_set_qobject(Object *obj, struct QObject *qobj,
> > const char *name, struct Error **errp);
> >
> > +/**
> > + * object_property_get_ptr:
> > + * @obj: the object
> > + * @ptr: The C struct that will be written to the property.
>
drop that line
> > + * @name: the name of the property
> > + * @visit_type: the visitor function for @ptr's type.
> > + * @errp: returns an error if this function fails
> > + *
> > + * Return: the value of an object's property, unmarshaled into a C
> object
> > + * through a QAPI type visitor, or NULL if an error occurs.
> > + */
> > +void *object_property_get_ptr(Object *obj, const char *name,
> > + void (*visit_type)(Visitor *, const char
> *,
> > + void **, Error **),
> > + Error **errp);
> > +
> > +/**
> > + * OBJECT_PROPERTY_GET_PTR:
> > + * @obj: the object
> > + * @ptr: The C struct that will be written to the property.
>
drop that line
> > + * @name: the name of the property
> > + * @type: the name of the C struct type pointed to by @ptr.
> > + * @errp: returns an error if this function fails
> > + *
> > + * Return: the value of an object's property, unmarshaled into a C
> object
> > + * through a QAPI type visitor, or NULL if an error occurs.
> > + */
> > +#define OBJECT_PROPERTY_GET_PTR(obj, name, type, errp)
> \
> > + ((type *)
> \
> > + object_property_get_ptr(obj, name,
> \
> > + (void (*)(Visitor *, const char *,
> void**, \
> > + Error **))visit_type_ ## type,
> \
> > + errp))
> > +
> > +/**
> > + * object_property_set_ptr:
> > + * @obj: the object
> > + * @ptr: The C struct that will be written to the property.
> > + * @name: the name of the property
> > + * @visit_type: the visitor function for @ptr's type.
> > + * @errp: returns an error if this function fails
> > + *
> > + * Sets an object's property to a C object's value, using a QAPI
> > + * type visitor to marshal the C struct into the object.
> > + */
> > +void object_property_set_ptr(Object *obj, void *ptr, const char *name,
> > + void (*visit_type)(Visitor *, const char *,
> > + void **, Error **),
> > + Error **errp);
> > +
> > +/**
> > + * OBJECT_PROPERTY_SET_PTR:
> > + * @obj: the object
> > + * @ptr: The C struct that will be written to the property.
> > + * @name: the name of the property
> > + * @type: the name of the C struct type pointed to by @ptr.
> > + * @errp: returns an error if this function fails
> > + *
> > + * Sets an object's property to a C object's value, using a QAPI
> > + * type visitor to marshal the C struct into the object.
> > + */
> > +#define OBJECT_PROPERTY_SET_PTR(obj, ptr, name, type, errp)
> \
> > + object_property_set_ptr(obj, ptr + type_check(type, typeof(*ptr)),
> \
> > + name,
> \
> > + (void (*)(Visitor *, const char *, void**,
> \
> > + Error **))visit_type_ ## type,
> \
> > + errp)
> > +
> > #endif
> > diff --git a/qom/qom-qobject.c b/qom/qom-qobject.c
> > index 447e4a0..09a12e0 100644
> > --- a/qom/qom-qobject.c
> > +++ b/qom/qom-qobject.c
> > @@ -44,3 +44,52 @@ QObject *object_property_get_qobject(Object *obj,
> const char *name,
> > visit_free(v);
> > return ret;
> > }
> > +
> > +void object_property_set_ptr(Object *obj, void *ptr, const char *name,
> > + void (*visit_type)(Visitor *, const char
> *, void **, Error **),
> > + Error **errp)
> > +{
> > + Error *local_err = NULL;
> > + QObject *ret = NULL;
> > + Visitor *v;
> > + v = qobject_output_visitor_new(&ret);
> > + visit_type(v, name, &ptr, &local_err);
> > + if (local_err) {
> > + error_propagate(errp, local_err);
> > + visit_free(v);
> > + return;
> > + }
> > + visit_complete(v, &ret);
> > + visit_free(v);
> > +
> > + /* Do not use object_property_set_qobject until we switch it
> > + * to use qobject_input_visitor_new in strict mode. See the
> > + * /qom/proplist/get-set-ptr/contravariant unit test.
> > + */
> > + v = qobject_input_visitor_new(ret, true);
> > + object_property_set(obj, v, name, errp);
> > + visit_free(v);
> > + qobject_decref(ret);
> > +}
> > +
> > +void *object_property_get_ptr(Object *obj, const char *name,
> > + void (*visit_type)(Visitor *, const char
> *, void **, Error **),
> > + Error **errp)
> > +{
> > + QObject *ret;
> > + Visitor *v;
> > + void *ptr = NULL;
> > +
> > + ret = object_property_get_qobject(obj, name, errp);
> > + if (!ret) {
> > + return NULL;
> > + }
> > +
> > + /* Do not enable strict mode to allow passing covariant
> > + * data types.
> > + */
> > + v = qobject_input_visitor_new(ret, false);
> > + visit_type(v, name, &ptr, errp);
> > + qobject_decref(ret);
>
visit_free(v) ?
> > + return ptr;
> > +}
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index 634394a..9de910b 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -515,7 +515,7 @@ tests/check-qfloat$(EXESUF): tests/check-qfloat.o
> $(test-util-obj-y)
> > tests/check-qnull$(EXESUF): tests/check-qnull.o $(test-util-obj-y)
> > tests/check-qjson$(EXESUF): tests/check-qjson.o $(test-util-obj-y)
> > tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o
> $(test-qom-obj-y)
> > -tests/check-qom-proplist$(EXESUF): tests/check-qom-proplist.o
> $(test-qom-obj-y)
> > +tests/check-qom-proplist$(EXESUF): tests/check-qom-proplist.o
> $(test-qom-obj-y) $(test-qapi-obj-y)
> >
> > tests/test-char$(EXESUF): tests/test-char.o qemu-timer.o \
> > $(test-util-obj-y) $(qtest-obj-y) $(test-block-obj-y)
> $(chardev-obj-y)
> > diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
> > index a16cefc..e0ad880 100644
> > --- a/tests/check-qom-proplist.c
> > +++ b/tests/check-qom-proplist.c
> > @@ -22,8 +22,11 @@
> >
> > #include "qapi/error.h"
> > #include "qom/object.h"
> > +#include "qom/qom-qobject.h"
> > #include "qemu/module.h"
> >
> > +#include "test-qapi-types.h"
> > +#include "test-qapi-visit.h"
> >
> > #define TYPE_DUMMY "qemu-dummy"
> >
> > @@ -56,6 +59,8 @@ struct DummyObject {
> > bool bv;
> > DummyAnimal av;
> > char *sv;
> > +
> > + UserDefOne *qv;
> > };
> >
> > struct DummyObjectClass {
> > @@ -120,12 +125,42 @@ static char *dummy_get_sv(Object *obj,
> >
> > static void dummy_init(Object *obj)
> > {
> > + DummyObject *dobj = DUMMY_OBJECT(obj);
> > +
> > object_property_add_bool(obj, "bv",
> > dummy_get_bv,
> > dummy_set_bv,
> > NULL);
> > + dobj->qv = g_new0(UserDefOne, 1);
> > + dobj->qv->string = g_strdup("dummy string");
> > +}
> > +
> > +
> > +static void dummy_get_qv(Object *obj, Visitor *v, const char *name,
> > + void *opaque, Error **errp)
> > +{
> > + DummyObject *dobj = DUMMY_OBJECT(obj);
> > +
> > + visit_type_UserDefOne(v, name, &dobj->qv, errp);
> > }
> >
> > +static void dummy_set_qv(Object *obj, Visitor *v, const char *name,
> > + void *opaque, Error **errp)
> > +{
> > + DummyObject *dobj = DUMMY_OBJECT(obj);
> > + UserDefOne *qv = NULL;
> > + Error *local_err = NULL;
> > +
> > + visit_type_UserDefOne(v, name, &qv, &local_err);
> > + if (local_err) {
> > + g_assert(qv == NULL);
> > + error_propagate(errp, local_err);
> > + return;
> > + }
> > +
> > + qapi_free_UserDefOne(dobj->qv);
> > + dobj->qv = qv;
> > +}
> >
> > static void dummy_class_init(ObjectClass *cls, void *data)
> > {
> > @@ -143,6 +178,13 @@ static void dummy_class_init(ObjectClass *cls, void
> *data)
> > dummy_get_av,
> > dummy_set_av,
> > NULL);
> > + object_class_property_add(cls, "qv",
> > + "UserDefOne",
> > + dummy_get_qv,
> > + dummy_set_qv,
> > + NULL,
> > + NULL,
> > + NULL);
> > }
> >
> >
> > @@ -151,9 +193,9 @@ static void dummy_finalize(Object *obj)
> > DummyObject *dobj = DUMMY_OBJECT(obj);
> >
> > g_free(dobj->sv);
> > + qapi_free_UserDefOne(dobj->qv);
> > }
> >
> > -
> > static const TypeInfo dummy_info = {
> > .name = TYPE_DUMMY,
> > .parent = TYPE_OBJECT,
> > @@ -473,7 +515,8 @@ static void test_dummy_iterator(void)
> >
> > ObjectProperty *prop;
> > ObjectPropertyIterator iter;
> > - bool seenbv = false, seensv = false, seenav = false, seentype;
> > + bool seenbv = false, seensv = false, seenav = false;
> > + bool seenqv = false, seentype = false;
> >
> > object_property_iter_init(&iter, OBJECT(dobj));
> > while ((prop = object_property_iter_next(&iter))) {
> > @@ -483,6 +526,8 @@ static void test_dummy_iterator(void)
> > seensv = true;
> > } else if (g_str_equal(prop->name, "av")) {
> > seenav = true;
> > + } else if (g_str_equal(prop->name, "qv")) {
> > + seenqv = true;
> > } else if (g_str_equal(prop->name, "type")) {
> > /* This prop comes from the base Object class */
> > seentype = true;
> > @@ -494,6 +539,7 @@ static void test_dummy_iterator(void)
> > g_assert(seenbv);
> > g_assert(seenav);
> > g_assert(seensv);
> > + g_assert(seenqv);
> > g_assert(seentype);
> >
> > object_unparent(OBJECT(dobj));
> > @@ -513,6 +559,137 @@ static void test_dummy_delchild(void)
> > object_unparent(OBJECT(dev));
> > }
> >
> > +static void test_dummy_get_set_ptr_struct(void)
> > +{
> > + DummyObject *dobj = DUMMY_OBJECT(object_new(TYPE_DUMMY));
> > + Error *local_err = NULL;
> > + const char *s = "my other dummy string";
> > + UserDefOne *ret;
> > + UserDefOne val;
> > +
> > + ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "qv",
> > + UserDefOne, &local_err);
> > + g_assert(!local_err);
> > +
> > + g_assert_cmpint(ret->integer, ==, 0);
> > + g_assert_cmpstr(ret->string, ==, "dummy string");
> > + g_assert(!ret->has_enum1);
> > + qapi_free_UserDefOne(ret);
> > +
> > + val.integer = 42;
> > + val.string = g_strdup(s);
> > + val.has_enum1 = true;
> > + val.enum1 = ENUM_ONE_VALUE1;
> > + OBJECT_PROPERTY_SET_PTR(OBJECT(dobj), &val, "qv",
> > + UserDefOne, &local_err);
> > + g_assert(!local_err);
> > +
> > + ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "qv",
> > + UserDefOne, &local_err);
> > + g_assert(!local_err);
> > +
> > + g_assert_cmpint(ret->integer, ==, val.integer);
> > + g_assert_cmpstr(ret->string, ==, val.string);
> > + g_assert(ret->has_enum1);
> > + g_assert_cmpint(ret->enum1, ==, val.enum1);
> > + g_free(val.string);
> > + qapi_free_UserDefOne(ret);
> > +}
> > +
> > +static void test_dummy_get_set_ptr_contravariant(void)
> > +{
> > + DummyObject *dobj = DUMMY_OBJECT(object_new(TYPE_DUMMY));
> > + Error *local_err = NULL;
> > + UserDefOneMore *ret;
> > + UserDefOneMore val;
> > +
> > + /* You cannot retrieve a contravariant (subclass) type... */
> > + ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "qv",
> > + UserDefOneMore, &local_err);
> > + g_assert(local_err);
> > + g_assert(!ret);
> > + error_free(local_err);
> > + local_err = NULL;
> > +
> > + /* And you cannot set one either. */
> > + val.integer = 42;
> > + val.string = g_strdup("unused");
> > + val.has_enum1 = false;
> > + val.boolean = false;
> > +
> > + OBJECT_PROPERTY_SET_PTR(OBJECT(dobj), &val, "qv",
> > + UserDefOneMore, &local_err);
> > + g_assert(local_err);
> > +}
> > +
> > +static void test_dummy_get_set_ptr_covariant(void)
> > +{
> > + DummyObject *dobj = DUMMY_OBJECT(object_new(TYPE_DUMMY));
> > + Error *local_err = NULL;
> > + UserDefZero *ret;
> > + UserDefZero val;
> > +
> > + /* You can retrieve a covariant (superclass) type... */
> > + ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "qv",
> > + UserDefZero, &local_err);
> > + g_assert(!local_err);
> > +
> > + g_assert_cmpint(ret->integer, ==, 0);
> > + qapi_free_UserDefZero(ret);
> > +
> > + /* But you cannot set one. */
> > + val.integer = 42;
> > + OBJECT_PROPERTY_SET_PTR(OBJECT(dobj), &val, "qv",
> > + UserDefZero, &local_err);
> > + g_assert(local_err);
> > + error_free(local_err);
> > + local_err = NULL;
> > +
> > + /* Test that the property has not been modified at all */
> > + ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "qv",
> > + UserDefZero, &local_err);
> > + g_assert(!local_err);
> > +
> > + g_assert_cmpint(ret->integer, ==, 0);
> > + qapi_free_UserDefZero(ret);
> > +}
> > +
> > +static void test_dummy_get_set_ptr_error(void)
> > +{
> > + DummyObject *dobj = DUMMY_OBJECT(object_new(TYPE_DUMMY));
> > + Error *local_err = NULL;
> > + const char *s = "my other dummy string";
> > + UserDefOne *ret;
> > + UserDefOne val;
> > +
> > + ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "blah",
> > + UserDefOne, &local_err);
> > + g_assert(local_err);
> > + g_assert(!ret);
> > + error_free(local_err);
> > + local_err = NULL;
> > +
> > + val.integer = 42;
> > + val.string = g_strdup(s);
> > + val.has_enum1 = true;
> > + val.enum1 = 100;
> > + OBJECT_PROPERTY_SET_PTR(OBJECT(dobj), &val, "qv",
> > + UserDefOne, &local_err);
> > + g_assert(local_err);
> > + error_free(local_err);
> > + local_err = NULL;
> > +
> > + ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "qv",
> > + UserDefOne, &local_err);
> > + g_assert(!local_err);
> > +
> > + /* Test that the property has not been modified at all */
> > + g_assert_cmpint(ret->integer, ==, 0);
> > + g_assert_cmpstr(ret->string, ==, "dummy string");
> > + g_assert(!ret->has_enum1);
> > + qapi_free_UserDefOne(ret);
> > +}
> > +
> > int main(int argc, char **argv)
> > {
> > g_test_init(&argc, &argv, NULL);
> > @@ -530,5 +707,9 @@ int main(int argc, char **argv)
> > g_test_add_func("/qom/proplist/iterator", test_dummy_iterator);
> > g_test_add_func("/qom/proplist/delchild", test_dummy_delchild);
> >
> > + g_test_add_func("/qom/proplist/get-set-ptr/struct",
> test_dummy_get_set_ptr_struct);
> > + g_test_add_func("/qom/proplist/get-set-ptr/error",
> test_dummy_get_set_ptr_error);
> > + g_test_add_func("/qom/proplist/get-set-ptr/covariant",
> test_dummy_get_set_ptr_covariant);
> > + g_test_add_func("/qom/proplist/get-set-ptr/contravariant",
> test_dummy_get_set_ptr_contravariant);
> > return g_test_run();
> > }
> > diff --git a/tests/qapi-schema/qapi-schema-test.json
> b/tests/qapi-schema/qapi-schema-test.json
> > index f4d8cc4..4e3f6ff 100644
> > --- a/tests/qapi-schema/qapi-schema-test.json
> > +++ b/tests/qapi-schema/qapi-schema-test.json
> > @@ -91,6 +91,14 @@
> > '*enum1': 'EnumOne' } } # intentional forward reference
> >
> > ##
> > +# @UserDefOneMore:
> > +# for testing nested structs
> > +##
> > +{ 'struct': 'UserDefOneMore',
> > + 'base': 'UserDefOne',
> > + 'data': { 'boolean': 'bool' } }
> > +
> > +##
> > # @EnumOne:
> > ##
> > { 'enum': 'EnumOne',
> >
>
> --
Marc-André Lureau
On 02/21/2017 04:42 AM, Paolo Bonzini wrote:
> The functions simplify the handling of QOM properties whose type
> is a QAPI struct. They go through a QObject just like the other
> functions that access a QOM property through its C type.
>
> Like QAPI_CLONE, the functions are wrapped by macros that take a
> QAPI type name and use it to build the name of a visitor function.
I'm glad I got QAPI_CLONE working - it has proven to be a nice source of
inspiration for further cleanups.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> include/qom/qom-qobject.h | 68 ++++++++++++
> qom/qom-qobject.c | 49 +++++++++
> tests/Makefile.include | 2 +-
> tests/check-qom-proplist.c | 185 +++++++++++++++++++++++++++++++-
> tests/qapi-schema/qapi-schema-test.json | 8 ++
> 5 files changed, 309 insertions(+), 3 deletions(-)
>
> diff --git a/include/qom/qom-qobject.h b/include/qom/qom-qobject.h
> index 77cd717..ff1d307 100644
> --- a/include/qom/qom-qobject.h
> +++ b/include/qom/qom-qobject.h
> @@ -39,4 +39,72 @@ struct QObject *object_property_get_qobject(Object *obj, const char *name,
> void object_property_set_qobject(Object *obj, struct QObject *qobj,
> const char *name, struct Error **errp);
>
> +/**
> + * object_property_get_ptr:
> + * @obj: the object
> + * @ptr: The C struct that will be written to the property.
Maybe: The C struct that will be set to the property's value
Except there is no ptr parameter in this function signature.
Copy-and-paste leftover?
> + * @name: the name of the property
Inconsistent use of trailing '.'
> + * @visit_type: the visitor function for @ptr's type.
Since there is no @ptr, this needs a touchup.
> + * @errp: returns an error if this function fails
> + *
> + * Return: the value of an object's property, unmarshaled into a C object
> + * through a QAPI type visitor, or NULL if an error occurs.
> + */
> +void *object_property_get_ptr(Object *obj, const char *name,
> + void (*visit_type)(Visitor *, const char *,
> + void **, Error **),
> + Error **errp);
> +
> +/**
> + * OBJECT_PROPERTY_GET_PTR:
> + * @obj: the object
> + * @ptr: The C struct that will be written to the property.
Again, no ptr parameter.
> + * @name: the name of the property
> + * @type: the name of the C struct type pointed to by @ptr.
and again
> + * @errp: returns an error if this function fails
> + *
> + * Return: the value of an object's property, unmarshaled into a C object
> + * through a QAPI type visitor, or NULL if an error occurs.
> + */
> +#define OBJECT_PROPERTY_GET_PTR(obj, name, type, errp) \
> + ((type *) \
> + object_property_get_ptr(obj, name, \
> + (void (*)(Visitor *, const char *, void**, \
> + Error **))visit_type_ ## type, \
Is it worth a typedef somewhere to make it easier to cast between
visitor functions?
> + errp))
> +
> +/**
> + * object_property_set_ptr:
> + * @obj: the object
> + * @ptr: The C struct that will be written to the property.
> + * @name: the name of the property
> + * @visit_type: the visitor function for @ptr's type.
It's a bit weird to separate 'ptr' and 'visit_type' by another
parameter, since they are so integrally related; can we reorder the
parameters to put 'name' before 'ptr', or does that look inconsistent
compared to other existing property manipulations?
> + * @errp: returns an error if this function fails
> + *
> + * Sets an object's property to a C object's value, using a QAPI
> + * type visitor to marshal the C struct into the object.
> + */
> +void object_property_set_ptr(Object *obj, void *ptr, const char *name,
> + void (*visit_type)(Visitor *, const char *,
> + void **, Error **),
> + Error **errp);
> +
> +/**
> + * OBJECT_PROPERTY_SET_PTR:
> + * @obj: the object
> + * @ptr: The C struct that will be written to the property.
> + * @name: the name of the property
> + * @type: the name of the C struct type pointed to by @ptr.
Same ordering question, same trailing '.' observation
> + * @errp: returns an error if this function fails
> + *
> + * Sets an object's property to a C object's value, using a QAPI
> + * type visitor to marshal the C struct into the object.
> + */
> +#define OBJECT_PROPERTY_SET_PTR(obj, ptr, name, type, errp) \
> + object_property_set_ptr(obj, ptr + type_check(type, typeof(*ptr)), \
> + name, \
> + (void (*)(Visitor *, const char *, void**, \
> + Error **))visit_type_ ## type, \
and the typedef'd function pointer may again come in handy.
> + errp)
> +
> #endif
> diff --git a/qom/qom-qobject.c b/qom/qom-qobject.c
> index 447e4a0..09a12e0 100644
> --- a/qom/qom-qobject.c
> +++ b/qom/qom-qobject.c
> @@ -44,3 +44,52 @@ QObject *object_property_get_qobject(Object *obj, const char *name,
> visit_free(v);
> return ret;
> }
> +
> +void object_property_set_ptr(Object *obj, void *ptr, const char *name,
> + void (*visit_type)(Visitor *, const char *, void **, Error **),
Long line; you wrapped it in the .h, so you could do so here, too.
> + Error **errp)
> +{
> + Error *local_err = NULL;
> + QObject *ret = NULL;
> + Visitor *v;
> + v = qobject_output_visitor_new(&ret);
> + visit_type(v, name, &ptr, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + visit_free(v);
> + return;
> + }
> + visit_complete(v, &ret);
> + visit_free(v);
> +
> + /* Do not use object_property_set_qobject until we switch it
> + * to use qobject_input_visitor_new in strict mode. See the
> + * /qom/proplist/get-set-ptr/contravariant unit test.
Interesting observation (and useful comment). I wonder how far we are
from killing off non-strict mode, but that's an independent question not
for this series.
> + */
> + v = qobject_input_visitor_new(ret, true);
> + object_property_set(obj, v, name, errp);
> + visit_free(v);
> + qobject_decref(ret);
> +}
> +
> +void *object_property_get_ptr(Object *obj, const char *name,
> + void (*visit_type)(Visitor *, const char *, void **, Error **),
> + Error **errp)
> +{
> + QObject *ret;
> + Visitor *v;
> + void *ptr = NULL;
> +
> + ret = object_property_get_qobject(obj, name, errp);
> + if (!ret) {
> + return NULL;
> + }
> +
> + /* Do not enable strict mode to allow passing covariant
> + * data types.
> + */
> + v = qobject_input_visitor_new(ret, false);
> + visit_type(v, name, &ptr, errp);
> + qobject_decref(ret);
> + return ptr;
> +}
> +++ b/tests/check-qom-proplist.c
> +
> +static void test_dummy_get_set_ptr_contravariant(void)
> +{
> + DummyObject *dobj = DUMMY_OBJECT(object_new(TYPE_DUMMY));
> + Error *local_err = NULL;
> + UserDefOneMore *ret;
> + UserDefOneMore val;
> +
> + /* You cannot retrieve a contravariant (subclass) type... */
> + ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "qv",
> + UserDefOneMore, &local_err);
> + g_assert(local_err);
> + g_assert(!ret);
> + error_free(local_err);
Markus just posted a cleanup patch that uses error_free_or_abort()
instead of multiple lines.
This check makes sense (we can't populate userDefOneMore, because it has
a mandatory field in the subtype that was not stored in the property).
> + local_err = NULL;
> +
> + /* And you cannot set one either. */
> + val.integer = 42;
> + val.string = g_strdup("unused");
> + val.has_enum1 = false;
> + val.boolean = false;
> +
> + OBJECT_PROPERTY_SET_PTR(OBJECT(dobj), &val, "qv",
> + UserDefOneMore, &local_err);
> + g_assert(local_err);
Leaks local_err; use error_free_or_abort().
Makes sense, because the subtype has an extra field that isn't supported
by the property.
> +}
> +
> +static void test_dummy_get_set_ptr_covariant(void)
> +{
> + DummyObject *dobj = DUMMY_OBJECT(object_new(TYPE_DUMMY));
> + Error *local_err = NULL;
> + UserDefZero *ret;
> + UserDefZero val;
> +
> + /* You can retrieve a covariant (superclass) type... */
> + ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "qv",
> + UserDefZero, &local_err);
> + g_assert(!local_err);
> +
> + g_assert_cmpint(ret->integer, ==, 0);
> + qapi_free_UserDefZero(ret);
You coded a non-strict visitor above to allow this to happen, but is it
really what we want? It basically means we are grabbing the property
fields we care about, while ignoring the rest of the property. I guess
it may be okay.
> +
> + /* But you cannot set one. */
> + val.integer = 42;
> + OBJECT_PROPERTY_SET_PTR(OBJECT(dobj), &val, "qv",
> + UserDefZero, &local_err);
> + g_assert(local_err);
> + error_free(local_err);
> + local_err = NULL;
Makes sense that we cannot set something that does not have enough
fields present to match what the property is expecting.
> +
> + /* Test that the property has not been modified at all */
> + ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "qv",
> + UserDefZero, &local_err);
> + g_assert(!local_err);
> +
> + g_assert_cmpint(ret->integer, ==, 0);
> + qapi_free_UserDefZero(ret);
> +}
> +
> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -91,6 +91,14 @@
> '*enum1': 'EnumOne' } } # intentional forward reference
>
> ##
> +# @UserDefOneMore:
> +# for testing nested structs
Is nested the right word here?
> +##
> +{ 'struct': 'UserDefOneMore',
> + 'base': 'UserDefOne',
> + 'data': { 'boolean': 'bool' } }
> +
> +##
> # @EnumOne:
> ##
> { 'enum': 'EnumOne',
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
On 21/02/2017 16:57, Eric Blake wrote: >> + /* You can retrieve a covariant (superclass) type... */ >> + ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "qv", >> + UserDefZero, &local_err); >> + g_assert(!local_err); >> + >> + g_assert_cmpint(ret->integer, ==, 0); >> + qapi_free_UserDefZero(ret); > > You coded a non-strict visitor above to allow this to happen, but is it > really what we want? It basically means we are grabbing the property > fields we care about, while ignoring the rest of the property. I guess > it may be okay. The tests were very useful to write, because I had hardly gotten any of the corner cases right. :) I think these semantics make the most sense. Yeah, it uses non-strict mode but this test provides a reason (namely, covariant return types) why non-strict mode is useful. It makes backwards-compatibility easier. >> + >> + /* Test that the property has not been modified at all */ >> + ret = OBJECT_PROPERTY_GET_PTR(OBJECT(dobj), "qv", >> + UserDefZero, &local_err); >> + g_assert(!local_err); >> + >> + g_assert_cmpint(ret->integer, ==, 0); >> + qapi_free_UserDefZero(ret); >> +} >> + >> +++ b/tests/qapi-schema/qapi-schema-test.json >> @@ -91,6 +91,14 @@ >> '*enum1': 'EnumOne' } } # intentional forward reference >> >> ## >> +# @UserDefOneMore: >> +# for testing nested structs > Is nested the right word here? Just copied from UserDefOne. :) I'll change both to "derived". Paolo
© 2016 - 2026 Red Hat, Inc.