All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] qom: reduce boilerplate required for declaring and defining objects
@ 2020-07-23 18:14 Daniel P. Berrangé
  2020-07-23 18:14 ` [PATCH 1/4] qom: make object_ref/unref use a void * instead of Object * Daniel P. Berrangé
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Daniel P. Berrangé @ 2020-07-23 18:14 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Daniel P. Berrangé, Eduardo Habkost

To just duplicate the patch 2 message....

When creating new QOM types, there is a lot of boilerplate code that
must be repeated using a standard pattern. This is tedious to write
and liable to suffer from subtle inconsistencies. Thus it would
benefit from some simple automation.

QOM was loosely inspired by GLib's GObject, and indeed GObject suffers
from the same burden of boilerplate code, but has long provided a set of
macros to eliminate this burden in the source implementation. More
recently it has also provided a set of macros to eliminate this burden
in the header declaration.

In GLib there are the G_DECLARE_* and G_DEFINE_* family of macros
for the header declaration and source implementation respectively:

  https://developer.gnome.org/gobject/stable/chapter-gobject.html
  https://developer.gnome.org/gobject/stable/howto-gobject.html

This patch takes inspiration from GObject to provide the equivalent
functionality for QOM.

In the header file, instead of:

    typedef struct MyDevice MyDevice;
    typedef struct MyDeviceClass MyDeviceClass;

    G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDeviceClass, object_unref)

    #define MY_DEVICE_GET_CLASS(void *obj) \
            OBJECT_GET_CLASS(MyDeviceClass, obj, TYPE_MY_DEVICE)
    #define MY_DEVICE_CLASS(void *klass) \
            OBJECT_CLASS_CHECK(MyDeviceClass, klass, TYPE_MY_DEVICE)
    #define MY_DEVICE(void *obj)
            OBJECT_CHECK(MyDevice, obj, TYPE_MY_DEVICE)

    struct MyDeviceClass {
        DeviceClass parent_class;
    };

We now have

    OBJECT_DECLARE_SIMPLE_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE)

In cases where the class needs some virtual methods, it can be left
to be implemented manually using

    OBJECT_DECLARE_TYPE(MyDevice, my_device, MY_DEVICE)

Note that these macros are including support for g_autoptr() for the
object types, which is something previously only supported for variables
declared as the base Object * type.

Meanwhile in the source file, instead of:

    static void my_device_finalize(Object *obj);
    static void my_device_class_init(ObjectClass *oc, void *data);
    static void my_device_init(Object *obj);

    static const TypeInfo my_device_info = {
        .parent = TYPE_DEVICE,
        .name = TYPE_MY_DEVICE,
        .instance_size = sizeof(MyDevice),
        .instance_init = my_device_init,
        .instance_finalize = my_device_finalize,
        .class_size = sizeof(MyDeviceClass),
        .class_init = my_device_class_init,
    };

    static void
    my_device_register_types(void)
    {
        type_register_static(&my_device_info);
    }
    type_init(my_device_register_types);

We now have

    OBJECT_DEFINE_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE)

Or, if a class needs to implement interfaces:

    OBJECT_DEFINE_TYPE_WITH_INTERFACES(MyDevice, my_device, MY_DEVICE, DEVICE,
                                       { TYPE_USER_CREATABLE }, { NULL })

Or, if a class needs to be abstract

    OBJECT_DEFINE_ABSTRACT_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE)

IOW, in both cases the maintainer now only has to think about the
interesting part of the code which implements useful functionality
and avoids much of the boilerplate.


Patches 3 and 4 illustrate the usage of the new macros, and by excluding
the qom changes, and just looking at the crypto, the diffstat shows the
benefits quite nicely:

 crypto/secret.c                 | 24 ++++--------------------
 crypto/secret_common.c          | 32 +++++++++-----------------------
 crypto/secret_keyring.c         | 28 +++++++++-------------------
 crypto/tlscreds.c               | 25 +++----------------------
 crypto/tlscredsanon.c           | 23 ++++-------------------
 crypto/tlscredspsk.c            | 25 +++++--------------------
 crypto/tlscredsx509.c           | 29 ++++-------------------------
 include/crypto/secret.h         | 11 ++---------
 include/crypto/secret_common.h  | 13 ++-----------
 include/crypto/secret_keyring.h | 18 ++----------------
 include/crypto/tlscreds.h       | 13 ++-----------
 include/crypto/tlscredsanon.h   | 14 ++------------
 include/crypto/tlscredspsk.h    | 13 ++-----------
 include/crypto/tlscredsx509.h   | 13 ++-----------
 14 files changed, 52 insertions(+), 229 deletions(-)

(The 'qom' file diffstat is misled by the large amount of API doc text
 added).

Daniel P. Berrangé (4):
  qom: make object_ref/unref use a void * instead of Object *.
  qom: provide convenient macros for declaring and defining types
  crypto: use QOM macros for declaration/definition of secret types
  crypto: use QOM macros for declaration/definition of TLS creds types

 crypto/secret.c                 |  24 +--
 crypto/secret_common.c          |  32 +---
 crypto/secret_keyring.c         |  28 +---
 crypto/tlscreds.c               |  25 +--
 crypto/tlscredsanon.c           |  23 +--
 crypto/tlscredspsk.c            |  25 +--
 crypto/tlscredsx509.c           |  29 +---
 include/crypto/secret.h         |  11 +-
 include/crypto/secret_common.h  |  13 +-
 include/crypto/secret_keyring.h |  18 +-
 include/crypto/tlscreds.h       |  13 +-
 include/crypto/tlscredsanon.h   |  14 +-
 include/crypto/tlscredspsk.h    |  13 +-
 include/crypto/tlscredsx509.h   |  13 +-
 include/qom/object.h            | 281 +++++++++++++++++++++++++++++++-
 qom/object.c                    |   6 +-
 16 files changed, 335 insertions(+), 233 deletions(-)

-- 
2.26.2




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

* [PATCH 1/4] qom: make object_ref/unref use a void * instead of Object *.
  2020-07-23 18:14 [PATCH 0/4] qom: reduce boilerplate required for declaring and defining objects Daniel P. Berrangé
@ 2020-07-23 18:14 ` Daniel P. Berrangé
  2020-07-23 19:04   ` Eric Blake
  2020-07-23 18:14 ` [PATCH 2/4] qom: provide convenient macros for declaring and defining types Daniel P. Berrangé
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 18+ messages in thread
From: Daniel P. Berrangé @ 2020-07-23 18:14 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Daniel P. Berrangé, Eduardo Habkost

The object_ref/unref methods are intended for use with any subclass of
the base Object. Using "Object *" in the signature is not adding any
meaningful level of type safety, since callers simply use "OBJECT(ptr)"
and this expands to an unchecked cast "(Object *)".

By using "void *" we enable the object_unref() method to be used to
provide support for g_autoptr() with any subclass.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/qom/object.h | 4 ++--
 qom/object.c         | 6 ++++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 0f3a60617c..1f8aa2d48e 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1035,7 +1035,7 @@ GSList *object_class_get_list_sorted(const char *implements_type,
  * as its reference count is greater than zero.
  * Returns: @obj
  */
-Object *object_ref(Object *obj);
+Object *object_ref(void *obj);
 
 /**
  * object_unref:
@@ -1044,7 +1044,7 @@ Object *object_ref(Object *obj);
  * Decrease the reference count of a object.  A object cannot be freed as long
  * as its reference count is greater than zero.
  */
-void object_unref(Object *obj);
+void object_unref(void *obj);
 
 /**
  * object_property_try_add:
diff --git a/qom/object.c b/qom/object.c
index 00fdf89b3b..b1822a2ef4 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1124,8 +1124,9 @@ GSList *object_class_get_list_sorted(const char *implements_type,
                         object_class_cmp);
 }
 
-Object *object_ref(Object *obj)
+Object *object_ref(void *objptr)
 {
+    Object *obj = OBJECT(objptr);
     if (!obj) {
         return NULL;
     }
@@ -1133,8 +1134,9 @@ Object *object_ref(Object *obj)
     return obj;
 }
 
-void object_unref(Object *obj)
+void object_unref(void *objptr)
 {
+    Object *obj = OBJECT(objptr);
     if (!obj) {
         return;
     }
-- 
2.26.2



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

* [PATCH 2/4] qom: provide convenient macros for declaring and defining types
  2020-07-23 18:14 [PATCH 0/4] qom: reduce boilerplate required for declaring and defining objects Daniel P. Berrangé
  2020-07-23 18:14 ` [PATCH 1/4] qom: make object_ref/unref use a void * instead of Object * Daniel P. Berrangé
@ 2020-07-23 18:14 ` Daniel P. Berrangé
  2020-07-23 19:23   ` Eric Blake
                     ` (2 more replies)
  2020-07-23 18:14 ` [PATCH 3/4] crypto: use QOM macros for declaration/definition of secret types Daniel P. Berrangé
  2020-07-23 18:14 ` [PATCH 4/4] crypto: use QOM macros for declaration/definition of TLS creds types Daniel P. Berrangé
  3 siblings, 3 replies; 18+ messages in thread
From: Daniel P. Berrangé @ 2020-07-23 18:14 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Daniel P. Berrangé, Eduardo Habkost

When creating new QOM types, there is a lot of boilerplate code that
must be repeated using a standard pattern. This is tedious to write
and liable to suffer from subtle inconsistencies. Thus it would
benefit from some simple automation.

QOM was loosely inspired by GLib's GObject, and indeed GObject suffers
from the same burden of boilerplate code, but has long provided a set of
macros to eliminate this burden in the source implementation. More
recently it has also provided a set of macros to eliminate this burden
in the header declaration.

In GLib there are the G_DECLARE_* and G_DEFINE_* family of macros
for the header declaration and source implementation respectively:

  https://developer.gnome.org/gobject/stable/chapter-gobject.html
  https://developer.gnome.org/gobject/stable/howto-gobject.html

This patch takes inspiration from GObject to provide the equivalent
functionality for QOM.

In the header file, instead of:

    typedef struct MyDevice MyDevice;
    typedef struct MyDeviceClass MyDeviceClass;

    G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDeviceClass, object_unref)

    #define MY_DEVICE_GET_CLASS(void *obj) \
            OBJECT_GET_CLASS(MyDeviceClass, obj, TYPE_MY_DEVICE)
    #define MY_DEVICE_CLASS(void *klass) \
            OBJECT_CLASS_CHECK(MyDeviceClass, klass, TYPE_MY_DEVICE)
    #define MY_DEVICE(void *obj)
            OBJECT_CHECK(MyDevice, obj, TYPE_MY_DEVICE)

    struct MyDeviceClass {
        DeviceClass parent_class;
    };

We now have

    OBJECT_DECLARE_SIMPLE_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE)

In cases where the class needs some virtual methods, it can be left
to be implemented manually using

    OBJECT_DECLARE_TYPE(MyDevice, my_device, MY_DEVICE)

Note that these macros are including support for g_autoptr() for the
object types, which is something previously only supported for variables
declared as the base Object * type.

Meanwhile in the source file, instead of:

    static void my_device_finalize(Object *obj);
    static void my_device_class_init(ObjectClass *oc, void *data);
    static void my_device_init(Object *obj);

    static const TypeInfo my_device_info = {
        .parent = TYPE_DEVICE,
        .name = TYPE_MY_DEVICE,
        .instance_size = sizeof(MyDevice),
        .instance_init = my_device_init,
        .instance_finalize = my_device_finalize,
        .class_size = sizeof(MyDeviceClass),
        .class_init = my_device_class_init,
    };

    static void
    my_device_register_types(void)
    {
        type_register_static(&my_device_info);
    }
    type_init(my_device_register_types);

We now have

    OBJECT_DEFINE_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE)

Or, if a class needs to implement interfaces:

    OBJECT_DEFINE_TYPE_WITH_INTERFACES(MyDevice, my_device, MY_DEVICE, DEVICE,
                                       { TYPE_USER_CREATABLE }, { NULL })

Or, if a class needs to be abstract

    OBJECT_DEFINE_ABSTRACT_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE)

IOW, in both cases the maintainer now only has to think about the
interesting part of the code which implements useful functionality
and avoids much of the boilerplate.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/qom/object.h | 277 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 277 insertions(+)

diff --git a/include/qom/object.h b/include/qom/object.h
index 1f8aa2d48e..be64421089 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -304,6 +304,119 @@ typedef struct InterfaceInfo InterfaceInfo;
  *
  * The first example of such a QOM method was #CPUClass.reset,
  * another example is #DeviceClass.realize.
+ *
+ * # Standard type declaration and definition macros #
+ *
+ * A lot of the code outlined above follows a standard pattern and naming
+ * convention. To reduce the amount of boilerplate code that needs to be
+ * written for a new type there are two sets of macros to generate the
+ * common parts in a standard format.
+ *
+ * A type is declared using the OBJECT_DECLARE macro family. In types
+ * which do not require any virtual functions in the class, the
+ * OBJECT_DECLARE_SIMPLE_TYPE macro is suitable, and is commonly placed
+ * in the header file:
+ *
+ * <example>
+ *   <title>Declaring a simple type</title>
+ *   <programlisting>
+ *     OBJECT_DECLARE_SIMPLE_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE)
+ *   </programlisting>
+ * </example>
+ *
+ * This is equivalent to the following:
+ *
+ * <example>
+ *   <title>Expansion from declaring a simple type</title>
+ *   <programlisting>
+ *     typedef struct MyDevice MyDevice;
+ *     typedef struct MyDeviceClass MyDeviceClass;
+ *
+ *     G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDeviceClass, object_unref)
+ *
+ *     #define MY_DEVICE_GET_CLASS(void *obj) \
+ *             OBJECT_GET_CLASS(MyDeviceClass, obj, TYPE_MY_DEVICE)
+ *     #define MY_DEVICE_CLASS(void *klass) \
+ *             OBJECT_CLASS_CHECK(MyDeviceClass, klass, TYPE_MY_DEVICE)
+ *     #define MY_DEVICE(void *obj)
+ *             OBJECT_CHECK(MyDevice, obj, TYPE_MY_DEVICE)
+ *
+ *     struct MyDeviceClass {
+ *         DeviceClass parent_class;
+ *     };
+ *   </programlisting>
+ * </example>
+ *
+ * The 'struct MyDevice' needs to be declared separately.
+ * If the type requires virtual functions to be declared in the class
+ * struct, then the alternative OBJECT_DECLARE_TYPE() macro can be
+ * used. This does the same as OBJECT_DECLARE_SIMPLE_TYPE(), but without
+ * the 'struct MyDeviceClass' definition.
+ *
+ * To implement the type, the OBJECT_DEFINE macro family is available.
+ * In the simple case the OBJECT_DEFINE_TYPE macro is suitable:
+ *
+ * <example>
+ *   <title>Defining a simple type</title>
+ *   <programlisting>
+ *     OBJECT_DEFINE_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE)
+ *   </programlisting>
+ * </example>
+ *
+ * This is equivalent to the following:
+ *
+ * <example>
+ *   <title>Expansion from defining a simple type</title>
+ *   <programlisting>
+ *     static void my_device_finalize(Object *obj);
+ *     static void my_device_class_init(ObjectClass *oc, void *data);
+ *     static void my_device_init(Object *obj);
+ *
+ *     static const TypeInfo my_device_info = {
+ *         .parent = TYPE_DEVICE,
+ *         .name = TYPE_MY_DEVICE,
+ *         .instance_size = sizeof(MyDevice),
+ *         .instance_init = my_device_init,
+ *         .instance_finalize = my_device_finalize,
+ *         .class_size = sizeof(MyDeviceClass),
+ *         .class_init = my_device_class_init,
+ *     };
+ *
+ *     static void
+ *     my_device_register_types(void)
+ *     {
+ *         type_register_static(&my_device_info);
+ *     }
+ *     type_init(my_device_register_types);
+ *   </programlisting>
+ * </example>
+ *
+ * This is sufficient to get the type registered with the type
+ * system, and the three standard methods now need to be implemented
+ * along with any other logic required for the type.
+ *
+ * If the type needs to implement one or more interfaces, then the
+ * OBJECT_DEFINE_TYPE_WITH_INTERFACES() macro can be used instead.
+ * This accepts an array of interface type names.
+ *
+ * <example>
+ *   <title>Defining a simple type implementing interfaces</title>
+ *   <programlisting>
+ *     OBJECT_DEFINE_TYPE_WITH_INTERFACES(MyDevice, my_device,
+ *                                        MY_DEVICE, DEVICE,
+ *                                        { TYPE_USER_CREATABLE }, { NULL })
+ *   </programlisting>
+ * </example>
+ *
+ * If the type is not intended to be instantiated, then then
+ * the OBJECT_DEFINE_ABSTRACT_TYPE() macro can be used instead:
+ *
+ * <example>
+ *   <title>Defining a simple type</title>
+ *   <programlisting>
+ *     OBJECT_DEFINE_ABSTRACT_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE)
+ *   </programlisting>
+ * </example>
  */
 
 
@@ -440,6 +553,170 @@ struct Object
     Object *parent;
 };
 
+/**
+ * OBJECT_DECLARE_TYPE:
+ * @ModuleObjName: the object name with initial capitalization
+ * @module_obj_name: the object name in lowercase with underscore separators
+ * @MODULE_OBJ_NAME: the object name in uppercase with underscore separators
+ *
+ * This macro is typically used in a header file, and will:
+ *
+ *   - create the typedefs for the object and class structs
+ *   - register the type for use with g_autoptr
+ *   - provide three standard type cast functions
+ *
+ * The object struct and class struct need to be declared manually.
+ */
+#define OBJECT_DECLARE_TYPE(ModuleObjName, module_obj_name, MODULE_OBJ_NAME) \
+    typedef struct ModuleObjName ModuleObjName; \
+    typedef struct ModuleObjName##Class ModuleObjName##Class; \
+    \
+    G_DEFINE_AUTOPTR_CLEANUP_FUNC(ModuleObjName##Class, object_unref) \
+    \
+    static inline G_GNUC_UNUSED ModuleObjName##Class * \
+    MODULE_OBJ_NAME##_GET_CLASS(void *obj) \
+    { return OBJECT_GET_CLASS(ModuleObjName##Class, obj, \
+                              TYPE_##MODULE_OBJ_NAME); } \
+    \
+    static inline G_GNUC_UNUSED ModuleObjName##Class * \
+    MODULE_OBJ_NAME##_CLASS(void *klass) \
+    { return OBJECT_CLASS_CHECK(ModuleObjName##Class, klass, \
+                                TYPE_##MODULE_OBJ_NAME); } \
+    \
+    static inline G_GNUC_UNUSED ModuleObjName * \
+    MODULE_OBJ_NAME(void *obj) \
+    { return OBJECT_CHECK(ModuleObjName, obj, \
+                          TYPE_##MODULE_OBJ_NAME); }
+
+/**
+ * OBJECT_DECLARE_SIMPLE_TYPE:
+ * @ModuleObjName: the object name with initial caps
+ * @module_obj_name: the object name in lowercase with underscore separators
+ * @MODULE_OBJ_NAME: the object name in uppercase with underscore separators
+ * @ParentModuleObjName: the parent object name with initial caps
+ *
+ * This does the same as OBJECT_DECLARE_TYPE(), but also declares
+ * the class struct, thus only the object struct needs to be declare
+ * manually.
+ *
+ * This macro should be used unless the class struct needs to have
+ * virtual methods declared.
+ */
+#define OBJECT_DECLARE_SIMPLE_TYPE(ModuleObjName, module_obj_name, \
+                                   MODULE_OBJ_NAME, ParentModuleObjName) \
+    OBJECT_DECLARE_TYPE(ModuleObjName, module_obj_name, MODULE_OBJ_NAME) \
+    struct ModuleObjName##Class { ParentModuleObjName##Class parent_class; };
+
+
+/**
+ * OBJECT_DEFINE_TYPE_EXTENDED:
+ * @ModuleObjName: the object name with initial caps
+ * @module_obj_name: the object name in lowercase with underscore separators
+ * @MODULE_OBJ_NAME: the object name in uppercase with underscore separators
+ * @PARENT_MODULE_OBJ_NAME: the parent object name in uppercase with underscore
+ *                          separators
+ * @ABSTRACT: boolean flag to indicate whether the object can be instantiated
+ * @...: list of initializers for "InterfaceInfo" to declare implemented interfaces
+ *
+ * This macro is typically used in a source file, and will:
+ *
+ *   - declare prototypes for _finalize, _class_init and _init methods
+ *   - declare the TypeInfo struct instance
+ *   - provide the constructor to register the type
+ *
+ * After using this macro, implementations of the _finalize, _class_init,
+ * and _init methods need to be written. Any of these can be zero-line
+ * no-op impls if no special logic is required for a given type.
+ *
+ * This macro should rarely be used, instead one of the more specialized
+ * macros is usually a better choice.
+ */
+#define OBJECT_DEFINE_TYPE_EXTENDED(ModuleObjName, module_obj_name, \
+                                    MODULE_OBJ_NAME, PARENT_MODULE_OBJ_NAME, \
+                                    ABSTRACT, ...) \
+    static void \
+    module_obj_name##_finalize(Object *obj); \
+    static void \
+    module_obj_name##_class_init(ObjectClass *oc, void *data); \
+    static void \
+    module_obj_name##_init(Object *obj); \
+    \
+    static const TypeInfo module_obj_name##_info = { \
+        .parent = TYPE_##PARENT_MODULE_OBJ_NAME, \
+        .name = TYPE_##MODULE_OBJ_NAME, \
+        .instance_size = sizeof(ModuleObjName), \
+        .instance_init = module_obj_name##_init, \
+        .instance_finalize = module_obj_name##_finalize, \
+        .class_size = sizeof(ModuleObjName##Class), \
+        .class_init = module_obj_name##_class_init, \
+        .abstract = ABSTRACT, \
+        .interfaces = (InterfaceInfo[]) { __VA_ARGS__ } , \
+    }; \
+    \
+    static void \
+    module_obj_name##_register_types(void) \
+    { \
+        type_register_static(&module_obj_name##_info); \
+    } \
+    type_init(module_obj_name##_register_types);
+
+/**
+ * OBJECT_DEFINE_TYPE:
+ * @ModuleObjName: the object name with initial caps
+ * @module_obj_name: the object name in lowercase with underscore separators
+ * @MODULE_OBJ_NAME: the object name in uppercase with underscore separators
+ * @PARENT_MODULE_OBJ_NAME: the parent object name in uppercase with underscore
+ *                          separators
+ *
+ * This is a specialization of OBJECT_DEFINE_TYPE_EXTENDED, which is suitable
+ * for the common case of a non-abstract type, without any interfaces.
+ */
+#define OBJECT_DEFINE_TYPE(ModuleObjName, module_obj_name, MODULE_OBJ_NAME, \
+                           PARENT_MODULE_OBJ_NAME) \
+    OBJECT_DEFINE_TYPE_EXTENDED(ModuleObjName, module_obj_name, \
+                                MODULE_OBJ_NAME, PARENT_MODULE_OBJ_NAME, \
+                                false, { NULL })
+
+/**
+ * OBJECT_DEFINE_TYPE_WITH_INTERFACES:
+ * @ModuleObjName: the object name with initial caps
+ * @module_obj_name: the object name in lowercase with underscore separators
+ * @MODULE_OBJ_NAME: the object name in uppercase with underscore separators
+ * @PARENT_MODULE_OBJ_NAME: the parent object name in uppercase with underscore
+ *                          separators
+ * @...: list of initializers for "InterfaceInfo" to declare implemented interfaces
+ *
+ * This is a specialization of OBJECT_DEFINE_TYPE_EXTENDED, which is suitable
+ * for the common case of a non-abstract type, with one or more implemented
+ * interfaces.
+ *
+ * Note when passing the list of interfaces, be sure to include the final
+ * NULL entry, e.g.  { TYPE_USER_CREATABLE }, { NULL }
+ */
+#define OBJECT_DEFINE_TYPE_WITH_INTERFACES(ModuleObjName, module_obj_name, \
+                                           MODULE_OBJ_NAME, \
+                                           PARENT_MODULE_OBJ_NAME, ...) \
+    OBJECT_DEFINE_TYPE_EXTENDED(ModuleObjName, module_obj_name, \
+                                MODULE_OBJ_NAME, PARENT_MODULE_OBJ_NAME, \
+                                false, __VA_ARGS__)
+
+/**
+ * OBJECT_DEFINE_ABSTRACT_TYPE:
+ * @ModuleObjName: the object name with initial caps
+ * @module_obj_name: the object name in lowercase with underscore separators
+ * @MODULE_OBJ_NAME: the object name in uppercase with underscore separators
+ * @PARENT_MODULE_OBJ_NAME: the parent object name in uppercase with underscore
+ *                          separators
+ *
+ * This is a specialization of OBJECT_DEFINE_TYPE_EXTENDED, which is suitable
+ * for defining an abstract type, without any interfaces.
+ */
+#define OBJECT_DEFINE_ABSTRACT_TYPE(ModuleObjName, module_obj_name, \
+                                    MODULE_OBJ_NAME, PARENT_MODULE_OBJ_NAME) \
+    OBJECT_DEFINE_TYPE_EXTENDED(ModuleObjName, module_obj_name, \
+                                MODULE_OBJ_NAME, PARENT_MODULE_OBJ_NAME, \
+                                true, { NULL })
+
 /**
  * TypeInfo:
  * @name: The name of the type.
-- 
2.26.2



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

* [PATCH 3/4] crypto: use QOM macros for declaration/definition of secret types
  2020-07-23 18:14 [PATCH 0/4] qom: reduce boilerplate required for declaring and defining objects Daniel P. Berrangé
  2020-07-23 18:14 ` [PATCH 1/4] qom: make object_ref/unref use a void * instead of Object * Daniel P. Berrangé
  2020-07-23 18:14 ` [PATCH 2/4] qom: provide convenient macros for declaring and defining types Daniel P. Berrangé
@ 2020-07-23 18:14 ` Daniel P. Berrangé
  2020-07-23 18:50   ` Eduardo Habkost
  2020-07-23 19:25   ` Eric Blake
  2020-07-23 18:14 ` [PATCH 4/4] crypto: use QOM macros for declaration/definition of TLS creds types Daniel P. Berrangé
  3 siblings, 2 replies; 18+ messages in thread
From: Daniel P. Berrangé @ 2020-07-23 18:14 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Daniel P. Berrangé, Eduardo Habkost

This introduces the use of the OBJECT_DEFINE and OBJECT_DECLARE macro
families in the secret types, in order to eliminate boilerplate code.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 crypto/secret.c                 | 24 ++++--------------------
 crypto/secret_common.c          | 32 +++++++++-----------------------
 crypto/secret_keyring.c         | 28 +++++++++-------------------
 include/crypto/secret.h         | 11 ++---------
 include/crypto/secret_common.h  | 13 ++-----------
 include/crypto/secret_keyring.h | 18 ++----------------
 6 files changed, 28 insertions(+), 98 deletions(-)

diff --git a/crypto/secret.c b/crypto/secret.c
index 281cb81f0f..55b406f79e 100644
--- a/crypto/secret.c
+++ b/crypto/secret.c
@@ -25,6 +25,9 @@
 #include "qemu/module.h"
 #include "trace.h"
 
+OBJECT_DEFINE_TYPE_WITH_INTERFACES(QCryptoSecret, qcrypto_secret,
+                                   QCRYPTO_SECRET, QCRYPTO_SECRET_COMMON,
+                                   { TYPE_USER_CREATABLE }, { NULL })
 
 static void
 qcrypto_secret_load_data(QCryptoSecretCommon *sec_common,
@@ -140,26 +143,7 @@ qcrypto_secret_class_init(ObjectClass *oc, void *data)
                                   qcrypto_secret_prop_set_file);
 }
 
-
-static const TypeInfo qcrypto_secret_info = {
-    .parent = TYPE_QCRYPTO_SECRET_COMMON,
-    .name = TYPE_QCRYPTO_SECRET,
-    .instance_size = sizeof(QCryptoSecret),
-    .instance_finalize = qcrypto_secret_finalize,
-    .class_size = sizeof(QCryptoSecretClass),
-    .class_init = qcrypto_secret_class_init,
-    .interfaces = (InterfaceInfo[]) {
-        { TYPE_USER_CREATABLE },
-        { }
-    }
-};
-
-
 static void
-qcrypto_secret_register_types(void)
+qcrypto_secret_init(Object *obj)
 {
-    type_register_static(&qcrypto_secret_info);
 }
-
-
-type_init(qcrypto_secret_register_types);
diff --git a/crypto/secret_common.c b/crypto/secret_common.c
index b03d530867..9a054b90b5 100644
--- a/crypto/secret_common.c
+++ b/crypto/secret_common.c
@@ -28,6 +28,9 @@
 #include "trace.h"
 
 
+OBJECT_DEFINE_ABSTRACT_TYPE(QCryptoSecretCommon, qcrypto_secret_common,
+                            QCRYPTO_SECRET_COMMON, OBJECT)
+
 static void qcrypto_secret_decrypt(QCryptoSecretCommon *secret,
                                    const uint8_t *input,
                                    size_t inputlen,
@@ -269,7 +272,7 @@ qcrypto_secret_prop_get_keyid(Object *obj,
 
 
 static void
-qcrypto_secret_finalize(Object *obj)
+qcrypto_secret_common_finalize(Object *obj)
 {
     QCryptoSecretCommon *secret = QCRYPTO_SECRET_COMMON(obj);
 
@@ -279,7 +282,7 @@ qcrypto_secret_finalize(Object *obj)
 }
 
 static void
-qcrypto_secret_class_init(ObjectClass *oc, void *data)
+qcrypto_secret_common_class_init(ObjectClass *oc, void *data)
 {
     object_class_property_add_bool(oc, "loaded",
                                    qcrypto_secret_prop_get_loaded,
@@ -297,6 +300,10 @@ qcrypto_secret_class_init(ObjectClass *oc, void *data)
                                   qcrypto_secret_prop_set_iv);
 }
 
+static void
+qcrypto_secret_common_init(Object *obj)
+{
+}
 
 int qcrypto_secret_lookup(const char *secretid,
                           uint8_t **data,
@@ -380,24 +387,3 @@ char *qcrypto_secret_lookup_as_base64(const char *secretid,
     g_free(data);
     return ret;
 }
-
-
-static const TypeInfo qcrypto_secret_info = {
-    .parent = TYPE_OBJECT,
-    .name = TYPE_QCRYPTO_SECRET_COMMON,
-    .instance_size = sizeof(QCryptoSecretCommon),
-    .instance_finalize = qcrypto_secret_finalize,
-    .class_size = sizeof(QCryptoSecretCommonClass),
-    .class_init = qcrypto_secret_class_init,
-    .abstract = true,
-};
-
-
-static void
-qcrypto_secret_register_types(void)
-{
-    type_register_static(&qcrypto_secret_info);
-}
-
-
-type_init(qcrypto_secret_register_types);
diff --git a/crypto/secret_keyring.c b/crypto/secret_keyring.c
index 8bfc58ebf4..463aefe5dc 100644
--- a/crypto/secret_keyring.c
+++ b/crypto/secret_keyring.c
@@ -26,6 +26,9 @@
 #include "trace.h"
 #include "crypto/secret_keyring.h"
 
+OBJECT_DEFINE_TYPE_WITH_INTERFACES(QCryptoSecretKeyring, qcrypto_secret_keyring,
+                                   QCRYPTO_SECRET_KEYRING, QCRYPTO_SECRET_COMMON,
+                                   { TYPE_USER_CREATABLE }, { NULL })
 
 static inline
 long keyctl_read(int32_t key, uint8_t *buffer, size_t buflen)
@@ -109,6 +112,11 @@ qcrypto_secret_keyring_complete(UserCreatable *uc, Error **errp)
 }
 
 
+static void
+qcrypto_secret_keyring_finalize(Object *obj)
+{
+}
+
 static void
 qcrypto_secret_keyring_class_init(ObjectClass *oc, void *data)
 {
@@ -124,25 +132,7 @@ qcrypto_secret_keyring_class_init(ObjectClass *oc, void *data)
                                   NULL, NULL);
 }
 
-
-static const TypeInfo qcrypto_secret_info = {
-    .parent = TYPE_QCRYPTO_SECRET_COMMON,
-    .name = TYPE_QCRYPTO_SECRET_KEYRING,
-    .instance_size = sizeof(QCryptoSecretKeyring),
-    .class_size = sizeof(QCryptoSecretKeyringClass),
-    .class_init = qcrypto_secret_keyring_class_init,
-    .interfaces = (InterfaceInfo[]) {
-        { TYPE_USER_CREATABLE },
-        { }
-    }
-};
-
-
 static void
-qcrypto_secret_register_types(void)
+qcrypto_secret_keyring_init(Object *obj)
 {
-    type_register_static(&qcrypto_secret_info);
 }
-
-
-type_init(qcrypto_secret_register_types);
diff --git a/include/crypto/secret.h b/include/crypto/secret.h
index 2deb461d2f..4eb4e5ffef 100644
--- a/include/crypto/secret.h
+++ b/include/crypto/secret.h
@@ -26,11 +26,9 @@
 #include "crypto/secret_common.h"
 
 #define TYPE_QCRYPTO_SECRET "secret"
-#define QCRYPTO_SECRET(obj)                  \
-    OBJECT_CHECK(QCryptoSecret, (obj), TYPE_QCRYPTO_SECRET)
 
-typedef struct QCryptoSecret QCryptoSecret;
-typedef struct QCryptoSecretClass QCryptoSecretClass;
+OBJECT_DECLARE_SIMPLE_TYPE(QCryptoSecret, qcrypto_secret,
+                           QCRYPTO_SECRET, QCryptoSecretCommon)
 
 /**
  * QCryptoSecret:
@@ -125,9 +123,4 @@ struct QCryptoSecret {
     char *file;
 };
 
-
-struct QCryptoSecretClass {
-    QCryptoSecretCommonClass parent_class;
-};
-
 #endif /* QCRYPTO_SECRET_H */
diff --git a/include/crypto/secret_common.h b/include/crypto/secret_common.h
index 980c02ab71..999a6b4651 100644
--- a/include/crypto/secret_common.h
+++ b/include/crypto/secret_common.h
@@ -25,17 +25,8 @@
 #include "qom/object.h"
 
 #define TYPE_QCRYPTO_SECRET_COMMON "secret_common"
-#define QCRYPTO_SECRET_COMMON(obj) \
-    OBJECT_CHECK(QCryptoSecretCommon, (obj), TYPE_QCRYPTO_SECRET_COMMON)
-#define QCRYPTO_SECRET_COMMON_CLASS(class) \
-    OBJECT_CLASS_CHECK(QCryptoSecretCommonClass, \
-                       (class), TYPE_QCRYPTO_SECRET_COMMON)
-#define QCRYPTO_SECRET_COMMON_GET_CLASS(obj) \
-    OBJECT_GET_CLASS(QCryptoSecretCommonClass, \
-                     (obj), TYPE_QCRYPTO_SECRET_COMMON)
-
-typedef struct QCryptoSecretCommon QCryptoSecretCommon;
-typedef struct QCryptoSecretCommonClass QCryptoSecretCommonClass;
+OBJECT_DECLARE_TYPE(QCryptoSecretCommon, qcrypto_secret_common,
+                    QCRYPTO_SECRET_COMMON)
 
 struct QCryptoSecretCommon {
     Object parent_obj;
diff --git a/include/crypto/secret_keyring.h b/include/crypto/secret_keyring.h
index 9f371ad251..4470306853 100644
--- a/include/crypto/secret_keyring.h
+++ b/include/crypto/secret_keyring.h
@@ -26,18 +26,8 @@
 #include "crypto/secret_common.h"
 
 #define TYPE_QCRYPTO_SECRET_KEYRING "secret_keyring"
-#define QCRYPTO_SECRET_KEYRING(obj) \
-    OBJECT_CHECK(QCryptoSecretKeyring, (obj), \
-                 TYPE_QCRYPTO_SECRET_KEYRING)
-#define QCRYPTO_SECRET_KEYRING_CLASS(class) \
-    OBJECT_CLASS_CHECK(QCryptoSecretKeyringClass, \
-                       (class), TYPE_QCRYPTO_SECRET_KEYRING)
-#define QCRYPTO_SECRET_KEYRING_GET_CLASS(class) \
-    OBJECT_GET_CLASS(QCryptoSecretKeyringClass, \
-                     (class), TYPE_QCRYPTO_SECRET_KEYRING)
-
-typedef struct QCryptoSecretKeyring QCryptoSecretKeyring;
-typedef struct QCryptoSecretKeyringClass QCryptoSecretKeyringClass;
+OBJECT_DECLARE_SIMPLE_TYPE(QCryptoSecretKeyring, qcrypto_secret_keyring,
+                           QCRYPTO_SECRET_KEYRING, QCryptoSecretCommon)
 
 typedef struct QCryptoSecretKeyring {
     QCryptoSecretCommon parent;
@@ -45,8 +35,4 @@ typedef struct QCryptoSecretKeyring {
 } QCryptoSecretKeyring;
 
 
-typedef struct QCryptoSecretKeyringClass {
-    QCryptoSecretCommonClass parent;
-} QCryptoSecretKeyringClass;
-
 #endif /* QCRYPTO_SECRET_KEYRING_H */
-- 
2.26.2



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

* [PATCH 4/4] crypto: use QOM macros for declaration/definition of TLS creds types
  2020-07-23 18:14 [PATCH 0/4] qom: reduce boilerplate required for declaring and defining objects Daniel P. Berrangé
                   ` (2 preceding siblings ...)
  2020-07-23 18:14 ` [PATCH 3/4] crypto: use QOM macros for declaration/definition of secret types Daniel P. Berrangé
@ 2020-07-23 18:14 ` Daniel P. Berrangé
  2020-07-23 19:26   ` Eric Blake
  3 siblings, 1 reply; 18+ messages in thread
From: Daniel P. Berrangé @ 2020-07-23 18:14 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Daniel P. Berrangé, Eduardo Habkost

This introduces the use of the OBJECT_DEFINE and OBJECT_DECLARE macro
families in the TLS creds types, in order to eliminate boilerplate code.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 crypto/tlscreds.c             | 25 +++----------------------
 crypto/tlscredsanon.c         | 23 ++++-------------------
 crypto/tlscredspsk.c          | 25 +++++--------------------
 crypto/tlscredsx509.c         | 29 ++++-------------------------
 include/crypto/tlscreds.h     | 13 ++-----------
 include/crypto/tlscredsanon.h | 14 ++------------
 include/crypto/tlscredspsk.h  | 13 ++-----------
 include/crypto/tlscredsx509.h | 13 ++-----------
 8 files changed, 24 insertions(+), 131 deletions(-)

diff --git a/crypto/tlscreds.c b/crypto/tlscreds.c
index b68735f06f..c238ff7d4b 100644
--- a/crypto/tlscreds.c
+++ b/crypto/tlscreds.c
@@ -24,6 +24,9 @@
 #include "tlscredspriv.h"
 #include "trace.h"
 
+OBJECT_DEFINE_ABSTRACT_TYPE(QCryptoTLSCreds, qcrypto_tls_creds,
+                            QCRYPTO_TLS_CREDS, OBJECT)
+
 #define DH_BITS 2048
 
 #ifdef CONFIG_GNUTLS
@@ -258,25 +261,3 @@ qcrypto_tls_creds_finalize(Object *obj)
     g_free(creds->dir);
     g_free(creds->priority);
 }
-
-
-static const TypeInfo qcrypto_tls_creds_info = {
-    .parent = TYPE_OBJECT,
-    .name = TYPE_QCRYPTO_TLS_CREDS,
-    .instance_size = sizeof(QCryptoTLSCreds),
-    .instance_init = qcrypto_tls_creds_init,
-    .instance_finalize = qcrypto_tls_creds_finalize,
-    .class_init = qcrypto_tls_creds_class_init,
-    .class_size = sizeof(QCryptoTLSCredsClass),
-    .abstract = true,
-};
-
-
-static void
-qcrypto_tls_creds_register_types(void)
-{
-    type_register_static(&qcrypto_tls_creds_info);
-}
-
-
-type_init(qcrypto_tls_creds_register_types);
diff --git a/crypto/tlscredsanon.c b/crypto/tlscredsanon.c
index 30275b6847..dc1b77e37c 100644
--- a/crypto/tlscredsanon.c
+++ b/crypto/tlscredsanon.c
@@ -26,6 +26,9 @@
 #include "qom/object_interfaces.h"
 #include "trace.h"
 
+OBJECT_DEFINE_TYPE_WITH_INTERFACES(QCryptoTLSCredsAnon, qcrypto_tls_creds_anon,
+                                   QCRYPTO_TLS_CREDS_ANON, QCRYPTO_TLS_CREDS,
+                                   { TYPE_USER_CREATABLE }, { NULL })
 
 #ifdef CONFIG_GNUTLS
 
@@ -191,25 +194,7 @@ qcrypto_tls_creds_anon_class_init(ObjectClass *oc, void *data)
 }
 
 
-static const TypeInfo qcrypto_tls_creds_anon_info = {
-    .parent = TYPE_QCRYPTO_TLS_CREDS,
-    .name = TYPE_QCRYPTO_TLS_CREDS_ANON,
-    .instance_size = sizeof(QCryptoTLSCredsAnon),
-    .instance_finalize = qcrypto_tls_creds_anon_finalize,
-    .class_size = sizeof(QCryptoTLSCredsAnonClass),
-    .class_init = qcrypto_tls_creds_anon_class_init,
-    .interfaces = (InterfaceInfo[]) {
-        { TYPE_USER_CREATABLE },
-        { }
-    }
-};
-
-
 static void
-qcrypto_tls_creds_anon_register_types(void)
+qcrypto_tls_creds_anon_init(Object *obj)
 {
-    type_register_static(&qcrypto_tls_creds_anon_info);
 }
-
-
-type_init(qcrypto_tls_creds_anon_register_types);
diff --git a/crypto/tlscredspsk.c b/crypto/tlscredspsk.c
index e26807b899..0c66be3647 100644
--- a/crypto/tlscredspsk.c
+++ b/crypto/tlscredspsk.c
@@ -27,6 +27,10 @@
 #include "trace.h"
 
 
+OBJECT_DEFINE_TYPE_WITH_INTERFACES(QCryptoTLSCredsPSK, qcrypto_tls_creds_psk,
+                                   QCRYPTO_TLS_CREDS_PSK, QCRYPTO_TLS_CREDS,
+                                   { TYPE_USER_CREATABLE }, { NULL })
+
 #ifdef CONFIG_GNUTLS
 
 static int
@@ -281,26 +285,7 @@ qcrypto_tls_creds_psk_class_init(ObjectClass *oc, void *data)
                                   qcrypto_tls_creds_psk_prop_set_username);
 }
 
-
-static const TypeInfo qcrypto_tls_creds_psk_info = {
-    .parent = TYPE_QCRYPTO_TLS_CREDS,
-    .name = TYPE_QCRYPTO_TLS_CREDS_PSK,
-    .instance_size = sizeof(QCryptoTLSCredsPSK),
-    .instance_finalize = qcrypto_tls_creds_psk_finalize,
-    .class_size = sizeof(QCryptoTLSCredsPSKClass),
-    .class_init = qcrypto_tls_creds_psk_class_init,
-    .interfaces = (InterfaceInfo[]) {
-        { TYPE_USER_CREATABLE },
-        { }
-    }
-};
-
-
 static void
-qcrypto_tls_creds_psk_register_types(void)
+qcrypto_tls_creds_psk_init(Object *obj)
 {
-    type_register_static(&qcrypto_tls_creds_psk_info);
 }
-
-
-type_init(qcrypto_tls_creds_psk_register_types);
diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index dd7267ccdb..a39555e5e6 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -28,6 +28,10 @@
 #include "trace.h"
 
 
+OBJECT_DEFINE_TYPE_WITH_INTERFACES(QCryptoTLSCredsX509, qcrypto_tls_creds_x509,
+                                   QCRYPTO_TLS_CREDS_X509, QCRYPTO_TLS_CREDS,
+                                   { TYPE_USER_CREATABLE }, { NULL })
+
 #ifdef CONFIG_GNUTLS
 
 #include <gnutls/x509.h>
@@ -814,28 +818,3 @@ qcrypto_tls_creds_x509_class_init(ObjectClass *oc, void *data)
                                   qcrypto_tls_creds_x509_prop_get_passwordid,
                                   qcrypto_tls_creds_x509_prop_set_passwordid);
 }
-
-
-static const TypeInfo qcrypto_tls_creds_x509_info = {
-    .parent = TYPE_QCRYPTO_TLS_CREDS,
-    .name = TYPE_QCRYPTO_TLS_CREDS_X509,
-    .instance_size = sizeof(QCryptoTLSCredsX509),
-    .instance_init = qcrypto_tls_creds_x509_init,
-    .instance_finalize = qcrypto_tls_creds_x509_finalize,
-    .class_size = sizeof(QCryptoTLSCredsX509Class),
-    .class_init = qcrypto_tls_creds_x509_class_init,
-    .interfaces = (InterfaceInfo[]) {
-        { TYPE_USER_CREATABLE },
-        { }
-    }
-};
-
-
-static void
-qcrypto_tls_creds_x509_register_types(void)
-{
-    type_register_static(&qcrypto_tls_creds_x509_info);
-}
-
-
-type_init(qcrypto_tls_creds_x509_register_types);
diff --git a/include/crypto/tlscreds.h b/include/crypto/tlscreds.h
index fd7a284aa2..e9b9b8c20a 100644
--- a/include/crypto/tlscreds.h
+++ b/include/crypto/tlscreds.h
@@ -29,11 +29,8 @@
 #endif
 
 #define TYPE_QCRYPTO_TLS_CREDS "tls-creds"
-#define QCRYPTO_TLS_CREDS(obj)                  \
-    OBJECT_CHECK(QCryptoTLSCreds, (obj), TYPE_QCRYPTO_TLS_CREDS)
-
-typedef struct QCryptoTLSCreds QCryptoTLSCreds;
-typedef struct QCryptoTLSCredsClass QCryptoTLSCredsClass;
+OBJECT_DECLARE_SIMPLE_TYPE(QCryptoTLSCreds, qcrypto_tls_creds,
+                           QCRYPTO_TLS_CREDS, Object)
 
 #define QCRYPTO_TLS_CREDS_DH_PARAMS "dh-params.pem"
 
@@ -58,10 +55,4 @@ struct QCryptoTLSCreds {
     char *priority;
 };
 
-
-struct QCryptoTLSCredsClass {
-    ObjectClass parent_class;
-};
-
-
 #endif /* QCRYPTO_TLSCREDS_H */
diff --git a/include/crypto/tlscredsanon.h b/include/crypto/tlscredsanon.h
index 9e9a5ce1a8..e366132b6b 100644
--- a/include/crypto/tlscredsanon.h
+++ b/include/crypto/tlscredsanon.h
@@ -24,12 +24,8 @@
 #include "crypto/tlscreds.h"
 
 #define TYPE_QCRYPTO_TLS_CREDS_ANON "tls-creds-anon"
-#define QCRYPTO_TLS_CREDS_ANON(obj)                  \
-    OBJECT_CHECK(QCryptoTLSCredsAnon, (obj), TYPE_QCRYPTO_TLS_CREDS_ANON)
-
-
-typedef struct QCryptoTLSCredsAnon QCryptoTLSCredsAnon;
-typedef struct QCryptoTLSCredsAnonClass QCryptoTLSCredsAnonClass;
+OBJECT_DECLARE_SIMPLE_TYPE(QCryptoTLSCredsAnon, qcrypto_tls_creds_anon,
+                           QCRYPTO_TLS_CREDS_ANON, QCryptoTLSCreds)
 
 /**
  * QCryptoTLSCredsAnon:
@@ -102,10 +98,4 @@ struct QCryptoTLSCredsAnon {
 #endif
 };
 
-
-struct QCryptoTLSCredsAnonClass {
-    QCryptoTLSCredsClass parent_class;
-};
-
-
 #endif /* QCRYPTO_TLSCREDSANON_H */
diff --git a/include/crypto/tlscredspsk.h b/include/crypto/tlscredspsk.h
index 907035a29b..0754c6c2d5 100644
--- a/include/crypto/tlscredspsk.h
+++ b/include/crypto/tlscredspsk.h
@@ -24,11 +24,8 @@
 #include "crypto/tlscreds.h"
 
 #define TYPE_QCRYPTO_TLS_CREDS_PSK "tls-creds-psk"
-#define QCRYPTO_TLS_CREDS_PSK(obj)                  \
-    OBJECT_CHECK(QCryptoTLSCredsPSK, (obj), TYPE_QCRYPTO_TLS_CREDS_PSK)
-
-typedef struct QCryptoTLSCredsPSK QCryptoTLSCredsPSK;
-typedef struct QCryptoTLSCredsPSKClass QCryptoTLSCredsPSKClass;
+OBJECT_DECLARE_SIMPLE_TYPE(QCryptoTLSCredsPSK, qcrypto_tls_creds_psk,
+                           QCRYPTO_TLS_CREDS_PSK, QCryptoTLSCreds)
 
 #define QCRYPTO_TLS_CREDS_PSKFILE "keys.psk"
 
@@ -97,10 +94,4 @@ struct QCryptoTLSCredsPSK {
 #endif
 };
 
-
-struct QCryptoTLSCredsPSKClass {
-    QCryptoTLSCredsClass parent_class;
-};
-
-
 #endif /* QCRYPTO_TLSCREDSPSK_H */
diff --git a/include/crypto/tlscredsx509.h b/include/crypto/tlscredsx509.h
index e1542e5c8c..307b169710 100644
--- a/include/crypto/tlscredsx509.h
+++ b/include/crypto/tlscredsx509.h
@@ -24,11 +24,8 @@
 #include "crypto/tlscreds.h"
 
 #define TYPE_QCRYPTO_TLS_CREDS_X509 "tls-creds-x509"
-#define QCRYPTO_TLS_CREDS_X509(obj)                  \
-    OBJECT_CHECK(QCryptoTLSCredsX509, (obj), TYPE_QCRYPTO_TLS_CREDS_X509)
-
-typedef struct QCryptoTLSCredsX509 QCryptoTLSCredsX509;
-typedef struct QCryptoTLSCredsX509Class QCryptoTLSCredsX509Class;
+OBJECT_DECLARE_SIMPLE_TYPE(QCryptoTLSCredsX509, qcrypto_tls_creds_x509,
+                           QCRYPTO_TLS_CREDS_X509, QCryptoTLSCreds)
 
 #define QCRYPTO_TLS_CREDS_X509_CA_CERT "ca-cert.pem"
 #define QCRYPTO_TLS_CREDS_X509_CA_CRL "ca-crl.pem"
@@ -104,10 +101,4 @@ struct QCryptoTLSCredsX509 {
     char *passwordid;
 };
 
-
-struct QCryptoTLSCredsX509Class {
-    QCryptoTLSCredsClass parent_class;
-};
-
-
 #endif /* QCRYPTO_TLSCREDSX509_H */
-- 
2.26.2



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

* Re: [PATCH 3/4] crypto: use QOM macros for declaration/definition of secret types
  2020-07-23 18:14 ` [PATCH 3/4] crypto: use QOM macros for declaration/definition of secret types Daniel P. Berrangé
@ 2020-07-23 18:50   ` Eduardo Habkost
  2020-07-24  9:12     ` Daniel P. Berrangé
  2020-07-23 19:25   ` Eric Blake
  1 sibling, 1 reply; 18+ messages in thread
From: Eduardo Habkost @ 2020-07-23 18:50 UTC (permalink / raw)
  To: Daniel P. Berrangé; +Cc: Paolo Bonzini, qemu-devel

On Thu, Jul 23, 2020 at 07:14:09PM +0100, Daniel P. Berrangé wrote:
> This introduces the use of the OBJECT_DEFINE and OBJECT_DECLARE macro
> families in the secret types, in order to eliminate boilerplate code.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  crypto/secret.c                 | 24 ++++--------------------
>  crypto/secret_common.c          | 32 +++++++++-----------------------
>  crypto/secret_keyring.c         | 28 +++++++++-------------------
>  include/crypto/secret.h         | 11 ++---------
>  include/crypto/secret_common.h  | 13 ++-----------
>  include/crypto/secret_keyring.h | 18 ++----------------
>  6 files changed, 28 insertions(+), 98 deletions(-)
> 

Beautiful.

I wonder how hard it would be to automate this.  I'm assuming
Coccinelle won't be able to deal with the macro definitions, but
a handwritten conversion script would be really useful for
dealing with our 1226 static TypeInfo structs.

-- 
Eduardo



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

* Re: [PATCH 1/4] qom: make object_ref/unref use a void * instead of Object *.
  2020-07-23 18:14 ` [PATCH 1/4] qom: make object_ref/unref use a void * instead of Object * Daniel P. Berrangé
@ 2020-07-23 19:04   ` Eric Blake
  2020-07-24  9:06     ` Daniel P. Berrangé
  0 siblings, 1 reply; 18+ messages in thread
From: Eric Blake @ 2020-07-23 19:04 UTC (permalink / raw)
  To: Daniel P. Berrangé, qemu-devel; +Cc: Paolo Bonzini, Eduardo Habkost

On 7/23/20 1:14 PM, Daniel P. Berrangé wrote:
> The object_ref/unref methods are intended for use with any subclass of
> the base Object. Using "Object *" in the signature is not adding any
> meaningful level of type safety, since callers simply use "OBJECT(ptr)"
> and this expands to an unchecked cast "(Object *)".
> 
> By using "void *" we enable the object_unref() method to be used to
> provide support for g_autoptr() with any subclass.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   include/qom/object.h | 4 ++--
>   qom/object.c         | 6 ++++--
>   2 files changed, 6 insertions(+), 4 deletions(-)

Is it worth a followup patch (probably with Coccinelle) that changes:

object_ref(OBJECT(dev));

to the now-simpler

object_ref(dev);

But I don't think it belongs in this patch, so

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



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

* Re: [PATCH 2/4] qom: provide convenient macros for declaring and defining types
  2020-07-23 18:14 ` [PATCH 2/4] qom: provide convenient macros for declaring and defining types Daniel P. Berrangé
@ 2020-07-23 19:23   ` Eric Blake
  2020-07-24  9:11     ` Daniel P. Berrangé
  2020-08-09  4:21   ` Eduardo Habkost
  2020-08-11 18:22   ` Eduardo Habkost
  2 siblings, 1 reply; 18+ messages in thread
From: Eric Blake @ 2020-07-23 19:23 UTC (permalink / raw)
  To: Daniel P. Berrangé, qemu-devel; +Cc: Paolo Bonzini, Eduardo Habkost

On 7/23/20 1:14 PM, Daniel P. Berrangé wrote:
> When creating new QOM types, there is a lot of boilerplate code that
> must be repeated using a standard pattern. This is tedious to write
> and liable to suffer from subtle inconsistencies. Thus it would
> benefit from some simple automation.
> 
> QOM was loosely inspired by GLib's GObject, and indeed GObject suffers
> from the same burden of boilerplate code, but has long provided a set of
> macros to eliminate this burden in the source implementation. More
> recently it has also provided a set of macros to eliminate this burden
> in the header declaration.
> 
> In GLib there are the G_DECLARE_* and G_DEFINE_* family of macros
> for the header declaration and source implementation respectively:
> 
>    https://developer.gnome.org/gobject/stable/chapter-gobject.html
>    https://developer.gnome.org/gobject/stable/howto-gobject.html
> 
> This patch takes inspiration from GObject to provide the equivalent
> functionality for QOM.
> 

> 
> IOW, in both cases the maintainer now only has to think about the
> interesting part of the code which implements useful functionality
> and avoids much of the boilerplate.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   include/qom/object.h | 277 +++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 277 insertions(+)
> 
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 1f8aa2d48e..be64421089 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -304,6 +304,119 @@ typedef struct InterfaceInfo InterfaceInfo;
>    *
>    * The first example of such a QOM method was #CPUClass.reset,
>    * another example is #DeviceClass.realize.
> + *
> + * # Standard type declaration and definition macros #
> + *
> + * A lot of the code outlined above follows a standard pattern and naming
> + * convention. To reduce the amount of boilerplate code that needs to be
> + * written for a new type there are two sets of macros to generate the
> + * common parts in a standard format.
> + *
> + * A type is declared using the OBJECT_DECLARE macro family. In types
> + * which do not require any virtual functions in the class, the
> + * OBJECT_DECLARE_SIMPLE_TYPE macro is suitable, and is commonly placed
> + * in the header file:
> + *
> + * <example>
> + *   <title>Declaring a simple type</title>
> + *   <programlisting>
> + *     OBJECT_DECLARE_SIMPLE_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE)

How sensitive is this macro to trailing semicolon?  Must the user omit 
it (as shown here), supply it (by tweaking the macro to be a syntax 
error if one is not supplied), or is it optional?  I guess whatever glib 
does is fine to copy, though.

Hmm. I think you meant to use s/ DEVICE/ Device/ here...

> + *   </programlisting>
> + * </example>
> + *
> + * This is equivalent to the following:
> + *
> + * <example>
> + *   <title>Expansion from declaring a simple type</title>
> + *   <programlisting>
> + *     typedef struct MyDevice MyDevice;
> + *     typedef struct MyDeviceClass MyDeviceClass;
> + *
> + *     G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDeviceClass, object_unref)
> + *
> + *     #define MY_DEVICE_GET_CLASS(void *obj) \
> + *             OBJECT_GET_CLASS(MyDeviceClass, obj, TYPE_MY_DEVICE)

How'd you manage to invoke #define inside the OBJECT_DECLARE_SIMPLE_TYPE 
macro expansion?

/me reads ahead

Oh, you didn't; you used a static inline function instead.  But the 
effect is the same, so claiming the equivalence here, while slightly 
misleading, is not horrible.

> + *     #define MY_DEVICE_CLASS(void *klass) \
> + *             OBJECT_CLASS_CHECK(MyDeviceClass, klass, TYPE_MY_DEVICE)
> + *     #define MY_DEVICE(void *obj)
> + *             OBJECT_CHECK(MyDevice, obj, TYPE_MY_DEVICE)
> + *
> + *     struct MyDeviceClass {
> + *         DeviceClass parent_class;

...given that this line is constructed as arg4##Class, and the fact that 
we have DeviceClass, not DEVICEClass.

> + *     };
> + *   </programlisting>
> + * </example>
> + *
> + * The 'struct MyDevice' needs to be declared separately.
> + * If the type requires virtual functions to be declared in the class
> + * struct, then the alternative OBJECT_DECLARE_TYPE() macro can be
> + * used. This does the same as OBJECT_DECLARE_SIMPLE_TYPE(), but without
> + * the 'struct MyDeviceClass' definition.
> + *
> + * To implement the type, the OBJECT_DEFINE macro family is available.
> + * In the simple case the OBJECT_DEFINE_TYPE macro is suitable:
> + *
> + * <example>
> + *   <title>Defining a simple type</title>
> + *   <programlisting>
> + *     OBJECT_DEFINE_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE)

Unlike the declare, here, using DEVICE looks correct...

> + *   </programlisting>
> + * </example>
> + *
> + * This is equivalent to the following:
> + *
> + * <example>
> + *   <title>Expansion from defining a simple type</title>
> + *   <programlisting>
> + *     static void my_device_finalize(Object *obj);
> + *     static void my_device_class_init(ObjectClass *oc, void *data);
> + *     static void my_device_init(Object *obj);
> + *
> + *     static const TypeInfo my_device_info = {
> + *         .parent = TYPE_DEVICE,

...given the expansion here.

> + *         .name = TYPE_MY_DEVICE,
> + *         .instance_size = sizeof(MyDevice),
> + *         .instance_init = my_device_init,
> + *         .instance_finalize = my_device_finalize,
> + *         .class_size = sizeof(MyDeviceClass),
> + *         .class_init = my_device_class_init,
> + *     };
> + *
> + *     static void
> + *     my_device_register_types(void)
> + *     {
> + *         type_register_static(&my_device_info);
> + *     }
> + *     type_init(my_device_register_types);
> + *   </programlisting>
> + * </example>
> + *
> + * This is sufficient to get the type registered with the type
> + * system, and the three standard methods now need to be implemented
> + * along with any other logic required for the type.
> + *
> + * If the type needs to implement one or more interfaces, then the
> + * OBJECT_DEFINE_TYPE_WITH_INTERFACES() macro can be used instead.
> + * This accepts an array of interface type names.
> + *
> + * <example>
> + *   <title>Defining a simple type implementing interfaces</title>
> + *   <programlisting>
> + *     OBJECT_DEFINE_TYPE_WITH_INTERFACES(MyDevice, my_device,
> + *                                        MY_DEVICE, DEVICE,
> + *                                        { TYPE_USER_CREATABLE }, { NULL })
> + *   </programlisting>
> + * </example>
> + *
> + * If the type is not intended to be instantiated, then then
> + * the OBJECT_DEFINE_ABSTRACT_TYPE() macro can be used instead:
> + *
> + * <example>
> + *   <title>Defining a simple type</title>

This title should probably mention 'abstract'.

> + *   <programlisting>
> + *     OBJECT_DEFINE_ABSTRACT_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE)
> + *   </programlisting>
> + * </example>
>    */
>   
>   
> @@ -440,6 +553,170 @@ struct Object
>       Object *parent;
>   };
>   
> +/**
> + * OBJECT_DECLARE_TYPE:
> + * @ModuleObjName: the object name with initial capitalization
> + * @module_obj_name: the object name in lowercase with underscore separators
> + * @MODULE_OBJ_NAME: the object name in uppercase with underscore separators
> + *
> + * This macro is typically used in a header file, and will:
> + *
> + *   - create the typedefs for the object and class structs
> + *   - register the type for use with g_autoptr
> + *   - provide three standard type cast functions
> + *
> + * The object struct and class struct need to be declared manually.
> + */
> +#define OBJECT_DECLARE_TYPE(ModuleObjName, module_obj_name, MODULE_OBJ_NAME) \
> +    typedef struct ModuleObjName ModuleObjName; \
> +    typedef struct ModuleObjName##Class ModuleObjName##Class; \
> +    \
> +    G_DEFINE_AUTOPTR_CLEANUP_FUNC(ModuleObjName##Class, object_unref) \
> +    \
> +    static inline G_GNUC_UNUSED ModuleObjName##Class * \
> +    MODULE_OBJ_NAME##_GET_CLASS(void *obj) \
> +    { return OBJECT_GET_CLASS(ModuleObjName##Class, obj, \
> +                              TYPE_##MODULE_OBJ_NAME); } \
> +    \
> +    static inline G_GNUC_UNUSED ModuleObjName##Class * \
> +    MODULE_OBJ_NAME##_CLASS(void *klass) \
> +    { return OBJECT_CLASS_CHECK(ModuleObjName##Class, klass, \
> +                                TYPE_##MODULE_OBJ_NAME); } \
> +    \
> +    static inline G_GNUC_UNUSED ModuleObjName * \
> +    MODULE_OBJ_NAME(void *obj) \
> +    { return OBJECT_CHECK(ModuleObjName, obj, \
> +                          TYPE_##MODULE_OBJ_NAME); }
> +
> +/**
> + * OBJECT_DECLARE_SIMPLE_TYPE:
> + * @ModuleObjName: the object name with initial caps
> + * @module_obj_name: the object name in lowercase with underscore separators
> + * @MODULE_OBJ_NAME: the object name in uppercase with underscore separators
> + * @ParentModuleObjName: the parent object name with initial caps
> + *
> + * This does the same as OBJECT_DECLARE_TYPE(), but also declares
> + * the class struct, thus only the object struct needs to be declare

declared

> + * manually.
> + *
> + * This macro should be used unless the class struct needs to have
> + * virtual methods declared.
> + */
> +#define OBJECT_DECLARE_SIMPLE_TYPE(ModuleObjName, module_obj_name, \
> +                                   MODULE_OBJ_NAME, ParentModuleObjName) \
> +    OBJECT_DECLARE_TYPE(ModuleObjName, module_obj_name, MODULE_OBJ_NAME) \
> +    struct ModuleObjName##Class { ParentModuleObjName##Class parent_class; };
> +
> +
> +/**
> + * OBJECT_DEFINE_TYPE_EXTENDED:
> + * @ModuleObjName: the object name with initial caps
> + * @module_obj_name: the object name in lowercase with underscore separators
> + * @MODULE_OBJ_NAME: the object name in uppercase with underscore separators
> + * @PARENT_MODULE_OBJ_NAME: the parent object name in uppercase with underscore
> + *                          separators
> + * @ABSTRACT: boolean flag to indicate whether the object can be instantiated
> + * @...: list of initializers for "InterfaceInfo" to declare implemented interfaces
> + *
> + * This macro is typically used in a source file, and will:
> + *
> + *   - declare prototypes for _finalize, _class_init and _init methods
> + *   - declare the TypeInfo struct instance
> + *   - provide the constructor to register the type
> + *
> + * After using this macro, implementations of the _finalize, _class_init,
> + * and _init methods need to be written. Any of these can be zero-line
> + * no-op impls if no special logic is required for a given type.
> + *
> + * This macro should rarely be used, instead one of the more specialized
> + * macros is usually a better choice.
> + */
> +#define OBJECT_DEFINE_TYPE_EXTENDED(ModuleObjName, module_obj_name, \
> +                                    MODULE_OBJ_NAME, PARENT_MODULE_OBJ_NAME, \
> +                                    ABSTRACT, ...) \
> +    static void \
> +    module_obj_name##_finalize(Object *obj); \
> +    static void \
> +    module_obj_name##_class_init(ObjectClass *oc, void *data); \
> +    static void \
> +    module_obj_name##_init(Object *obj); \
> +    \
> +    static const TypeInfo module_obj_name##_info = { \
> +        .parent = TYPE_##PARENT_MODULE_OBJ_NAME, \
> +        .name = TYPE_##MODULE_OBJ_NAME, \
> +        .instance_size = sizeof(ModuleObjName), \
> +        .instance_init = module_obj_name##_init, \
> +        .instance_finalize = module_obj_name##_finalize, \
> +        .class_size = sizeof(ModuleObjName##Class), \
> +        .class_init = module_obj_name##_class_init, \
> +        .abstract = ABSTRACT, \
> +        .interfaces = (InterfaceInfo[]) { __VA_ARGS__ } , \

Odd space before comma.

> +    }; \
> +    \
> +    static void \
> +    module_obj_name##_register_types(void) \
> +    { \
> +        type_register_static(&module_obj_name##_info); \
> +    } \
> +    type_init(module_obj_name##_register_types);
> +
> +/**
> + * OBJECT_DEFINE_TYPE:
> + * @ModuleObjName: the object name with initial caps
> + * @module_obj_name: the object name in lowercase with underscore separators
> + * @MODULE_OBJ_NAME: the object name in uppercase with underscore separators
> + * @PARENT_MODULE_OBJ_NAME: the parent object name in uppercase with underscore
> + *                          separators
> + *
> + * This is a specialization of OBJECT_DEFINE_TYPE_EXTENDED, which is suitable
> + * for the common case of a non-abstract type, without any interfaces.
> + */
> +#define OBJECT_DEFINE_TYPE(ModuleObjName, module_obj_name, MODULE_OBJ_NAME, \
> +                           PARENT_MODULE_OBJ_NAME) \
> +    OBJECT_DEFINE_TYPE_EXTENDED(ModuleObjName, module_obj_name, \
> +                                MODULE_OBJ_NAME, PARENT_MODULE_OBJ_NAME, \
> +                                false, { NULL })
> +
> +/**
> + * OBJECT_DEFINE_TYPE_WITH_INTERFACES:
> + * @ModuleObjName: the object name with initial caps
> + * @module_obj_name: the object name in lowercase with underscore separators
> + * @MODULE_OBJ_NAME: the object name in uppercase with underscore separators
> + * @PARENT_MODULE_OBJ_NAME: the parent object name in uppercase with underscore
> + *                          separators
> + * @...: list of initializers for "InterfaceInfo" to declare implemented interfaces
> + *
> + * This is a specialization of OBJECT_DEFINE_TYPE_EXTENDED, which is suitable
> + * for the common case of a non-abstract type, with one or more implemented
> + * interfaces.
> + *
> + * Note when passing the list of interfaces, be sure to include the final
> + * NULL entry, e.g.  { TYPE_USER_CREATABLE }, { NULL }
> + */
> +#define OBJECT_DEFINE_TYPE_WITH_INTERFACES(ModuleObjName, module_obj_name, \
> +                                           MODULE_OBJ_NAME, \
> +                                           PARENT_MODULE_OBJ_NAME, ...) \
> +    OBJECT_DEFINE_TYPE_EXTENDED(ModuleObjName, module_obj_name, \
> +                                MODULE_OBJ_NAME, PARENT_MODULE_OBJ_NAME, \
> +                                false, __VA_ARGS__)
> +
> +/**
> + * OBJECT_DEFINE_ABSTRACT_TYPE:
> + * @ModuleObjName: the object name with initial caps
> + * @module_obj_name: the object name in lowercase with underscore separators
> + * @MODULE_OBJ_NAME: the object name in uppercase with underscore separators
> + * @PARENT_MODULE_OBJ_NAME: the parent object name in uppercase with underscore
> + *                          separators
> + *
> + * This is a specialization of OBJECT_DEFINE_TYPE_EXTENDED, which is suitable
> + * for defining an abstract type, without any interfaces.
> + */
> +#define OBJECT_DEFINE_ABSTRACT_TYPE(ModuleObjName, module_obj_name, \
> +                                    MODULE_OBJ_NAME, PARENT_MODULE_OBJ_NAME) \
> +    OBJECT_DEFINE_TYPE_EXTENDED(ModuleObjName, module_obj_name, \
> +                                MODULE_OBJ_NAME, PARENT_MODULE_OBJ_NAME, \
> +                                true, { NULL })
> +
>   /**
>    * TypeInfo:
>    * @name: The name of the type.
> 

A couple minor spelling findings, but overall looks good.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



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

* Re: [PATCH 3/4] crypto: use QOM macros for declaration/definition of secret types
  2020-07-23 18:14 ` [PATCH 3/4] crypto: use QOM macros for declaration/definition of secret types Daniel P. Berrangé
  2020-07-23 18:50   ` Eduardo Habkost
@ 2020-07-23 19:25   ` Eric Blake
  1 sibling, 0 replies; 18+ messages in thread
From: Eric Blake @ 2020-07-23 19:25 UTC (permalink / raw)
  To: Daniel P. Berrangé, qemu-devel; +Cc: Paolo Bonzini, Eduardo Habkost

On 7/23/20 1:14 PM, Daniel P. Berrangé wrote:
> This introduces the use of the OBJECT_DEFINE and OBJECT_DECLARE macro
> families in the secret types, in order to eliminate boilerplate code.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



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

* Re: [PATCH 4/4] crypto: use QOM macros for declaration/definition of TLS creds types
  2020-07-23 18:14 ` [PATCH 4/4] crypto: use QOM macros for declaration/definition of TLS creds types Daniel P. Berrangé
@ 2020-07-23 19:26   ` Eric Blake
  0 siblings, 0 replies; 18+ messages in thread
From: Eric Blake @ 2020-07-23 19:26 UTC (permalink / raw)
  To: Daniel P. Berrangé, qemu-devel; +Cc: Paolo Bonzini, Eduardo Habkost

On 7/23/20 1:14 PM, Daniel P. Berrangé wrote:
> This introduces the use of the OBJECT_DEFINE and OBJECT_DECLARE macro
> families in the TLS creds types, in order to eliminate boilerplate code.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



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

* Re: [PATCH 1/4] qom: make object_ref/unref use a void * instead of Object *.
  2020-07-23 19:04   ` Eric Blake
@ 2020-07-24  9:06     ` Daniel P. Berrangé
  0 siblings, 0 replies; 18+ messages in thread
From: Daniel P. Berrangé @ 2020-07-24  9:06 UTC (permalink / raw)
  To: Eric Blake; +Cc: Paolo Bonzini, qemu-devel, Eduardo Habkost

On Thu, Jul 23, 2020 at 02:04:45PM -0500, Eric Blake wrote:
> On 7/23/20 1:14 PM, Daniel P. Berrangé wrote:
> > The object_ref/unref methods are intended for use with any subclass of
> > the base Object. Using "Object *" in the signature is not adding any
> > meaningful level of type safety, since callers simply use "OBJECT(ptr)"
> > and this expands to an unchecked cast "(Object *)".
> > 
> > By using "void *" we enable the object_unref() method to be used to
> > provide support for g_autoptr() with any subclass.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >   include/qom/object.h | 4 ++--
> >   qom/object.c         | 6 ++++--
> >   2 files changed, 6 insertions(+), 4 deletions(-)
> 
> Is it worth a followup patch (probably with Coccinelle) that changes:
> 
> object_ref(OBJECT(dev));
> 
> to the now-simpler
> 
> object_ref(dev);

Yes, its worth a cleanup.

> But I don't think it belongs in this patch, so
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



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

* Re: [PATCH 2/4] qom: provide convenient macros for declaring and defining types
  2020-07-23 19:23   ` Eric Blake
@ 2020-07-24  9:11     ` Daniel P. Berrangé
  0 siblings, 0 replies; 18+ messages in thread
From: Daniel P. Berrangé @ 2020-07-24  9:11 UTC (permalink / raw)
  To: Eric Blake; +Cc: Paolo Bonzini, qemu-devel, Eduardo Habkost

On Thu, Jul 23, 2020 at 02:23:54PM -0500, Eric Blake wrote:
> On 7/23/20 1:14 PM, Daniel P. Berrangé wrote:
> > When creating new QOM types, there is a lot of boilerplate code that
> > must be repeated using a standard pattern. This is tedious to write
> > and liable to suffer from subtle inconsistencies. Thus it would
> > benefit from some simple automation.
> > 
> > QOM was loosely inspired by GLib's GObject, and indeed GObject suffers
> > from the same burden of boilerplate code, but has long provided a set of
> > macros to eliminate this burden in the source implementation. More
> > recently it has also provided a set of macros to eliminate this burden
> > in the header declaration.
> > 
> > In GLib there are the G_DECLARE_* and G_DEFINE_* family of macros
> > for the header declaration and source implementation respectively:
> > 
> >    https://developer.gnome.org/gobject/stable/chapter-gobject.html
> >    https://developer.gnome.org/gobject/stable/howto-gobject.html
> > 
> > This patch takes inspiration from GObject to provide the equivalent
> > functionality for QOM.
> > 
> 
> > 
> > IOW, in both cases the maintainer now only has to think about the
> > interesting part of the code which implements useful functionality
> > and avoids much of the boilerplate.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >   include/qom/object.h | 277 +++++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 277 insertions(+)
> > 
> > diff --git a/include/qom/object.h b/include/qom/object.h
> > index 1f8aa2d48e..be64421089 100644
> > --- a/include/qom/object.h
> > +++ b/include/qom/object.h
> > @@ -304,6 +304,119 @@ typedef struct InterfaceInfo InterfaceInfo;
> >    *
> >    * The first example of such a QOM method was #CPUClass.reset,
> >    * another example is #DeviceClass.realize.
> > + *
> > + * # Standard type declaration and definition macros #
> > + *
> > + * A lot of the code outlined above follows a standard pattern and naming
> > + * convention. To reduce the amount of boilerplate code that needs to be
> > + * written for a new type there are two sets of macros to generate the
> > + * common parts in a standard format.
> > + *
> > + * A type is declared using the OBJECT_DECLARE macro family. In types
> > + * which do not require any virtual functions in the class, the
> > + * OBJECT_DECLARE_SIMPLE_TYPE macro is suitable, and is commonly placed
> > + * in the header file:
> > + *
> > + * <example>
> > + *   <title>Declaring a simple type</title>
> > + *   <programlisting>
> > + *     OBJECT_DECLARE_SIMPLE_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE)
> 
> How sensitive is this macro to trailing semicolon?  Must the user omit it
> (as shown here), supply it (by tweaking the macro to be a syntax error if
> one is not supplied), or is it optional?  I guess whatever glib does is fine
> to copy, though.

Testing it appears it tolerates a ";" but GLib doesn't use it typically
in this case.

> 
> Hmm. I think you meant to use s/ DEVICE/ Device/ here...

Yes.

> 
> > + *   </programlisting>
> > + * </example>
> > + *
> > + * This is equivalent to the following:
> > + *
> > + * <example>
> > + *   <title>Expansion from declaring a simple type</title>
> > + *   <programlisting>
> > + *     typedef struct MyDevice MyDevice;
> > + *     typedef struct MyDeviceClass MyDeviceClass;
> > + *
> > + *     G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDeviceClass, object_unref)
> > + *
> > + *     #define MY_DEVICE_GET_CLASS(void *obj) \
> > + *             OBJECT_GET_CLASS(MyDeviceClass, obj, TYPE_MY_DEVICE)
> 
> How'd you manage to invoke #define inside the OBJECT_DECLARE_SIMPLE_TYPE
> macro expansion?
> 
> /me reads ahead
> 
> Oh, you didn't; you used a static inline function instead.  But the effect
> is the same, so claiming the equivalence here, while slightly misleading, is
> not horrible.

Yes, I was in two minds here, whether to illustrate the real inline
function, or the macro. I choose the macro to make it clear what kind
of code is being replaced, rather than what kind of code it strictly
produces.

> > + *     #define MY_DEVICE_CLASS(void *klass) \
> > + *             OBJECT_CLASS_CHECK(MyDeviceClass, klass, TYPE_MY_DEVICE)
> > + *     #define MY_DEVICE(void *obj)
> > + *             OBJECT_CHECK(MyDevice, obj, TYPE_MY_DEVICE)
> > + *
> > + *     struct MyDeviceClass {
> > + *         DeviceClass parent_class;
> 
> ...given that this line is constructed as arg4##Class, and the fact that we
> have DeviceClass, not DEVICEClass.
> 
> > + *     };
> > + *   </programlisting>
> > + * </example>
> > + *
> > + * The 'struct MyDevice' needs to be declared separately.
> > + * If the type requires virtual functions to be declared in the class
> > + * struct, then the alternative OBJECT_DECLARE_TYPE() macro can be
> > + * used. This does the same as OBJECT_DECLARE_SIMPLE_TYPE(), but without
> > + * the 'struct MyDeviceClass' definition.
> > + *
> > + * To implement the type, the OBJECT_DEFINE macro family is available.
> > + * In the simple case the OBJECT_DEFINE_TYPE macro is suitable:
> > + *
> > + * <example>
> > + *   <title>Defining a simple type</title>
> > + *   <programlisting>
> > + *     OBJECT_DEFINE_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE)
> 
> Unlike the declare, here, using DEVICE looks correct...

Yes.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



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

* Re: [PATCH 3/4] crypto: use QOM macros for declaration/definition of secret types
  2020-07-23 18:50   ` Eduardo Habkost
@ 2020-07-24  9:12     ` Daniel P. Berrangé
  2020-08-06 18:01       ` Eduardo Habkost
  0 siblings, 1 reply; 18+ messages in thread
From: Daniel P. Berrangé @ 2020-07-24  9:12 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: Paolo Bonzini, qemu-devel

On Thu, Jul 23, 2020 at 02:50:06PM -0400, Eduardo Habkost wrote:
> On Thu, Jul 23, 2020 at 07:14:09PM +0100, Daniel P. Berrangé wrote:
> > This introduces the use of the OBJECT_DEFINE and OBJECT_DECLARE macro
> > families in the secret types, in order to eliminate boilerplate code.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  crypto/secret.c                 | 24 ++++--------------------
> >  crypto/secret_common.c          | 32 +++++++++-----------------------
> >  crypto/secret_keyring.c         | 28 +++++++++-------------------
> >  include/crypto/secret.h         | 11 ++---------
> >  include/crypto/secret_common.h  | 13 ++-----------
> >  include/crypto/secret_keyring.h | 18 ++----------------
> >  6 files changed, 28 insertions(+), 98 deletions(-)
> > 
> 
> Beautiful.
> 
> I wonder how hard it would be to automate this.  I'm assuming
> Coccinelle won't be able to deal with the macro definitions, but
> a handwritten conversion script would be really useful for
> dealing with our 1226 static TypeInfo structs.

Probably possible to do a reasonably good job with a perl script or
similar. The code patterns to be replaced are reasonably easy to
identify with a few regexes.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



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

* Re: [PATCH 3/4] crypto: use QOM macros for declaration/definition of secret types
  2020-07-24  9:12     ` Daniel P. Berrangé
@ 2020-08-06 18:01       ` Eduardo Habkost
  2020-08-07 11:11         ` Daniel P. Berrangé
  0 siblings, 1 reply; 18+ messages in thread
From: Eduardo Habkost @ 2020-08-06 18:01 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Peter Maydell, Paolo Bonzini, qemu-devel, Markus Armbruster

On Fri, Jul 24, 2020 at 10:12:45AM +0100, Daniel P. Berrangé wrote:
> On Thu, Jul 23, 2020 at 02:50:06PM -0400, Eduardo Habkost wrote:
> > On Thu, Jul 23, 2020 at 07:14:09PM +0100, Daniel P. Berrangé wrote:
> > > This introduces the use of the OBJECT_DEFINE and OBJECT_DECLARE macro
> > > families in the secret types, in order to eliminate boilerplate code.
> > > 
> > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > > ---
> > >  crypto/secret.c                 | 24 ++++--------------------
> > >  crypto/secret_common.c          | 32 +++++++++-----------------------
> > >  crypto/secret_keyring.c         | 28 +++++++++-------------------
> > >  include/crypto/secret.h         | 11 ++---------
> > >  include/crypto/secret_common.h  | 13 ++-----------
> > >  include/crypto/secret_keyring.h | 18 ++----------------
> > >  6 files changed, 28 insertions(+), 98 deletions(-)
> > > 
> > 
> > Beautiful.
> > 
> > I wonder how hard it would be to automate this.  I'm assuming
> > Coccinelle won't be able to deal with the macro definitions, but
> > a handwritten conversion script would be really useful for
> > dealing with our 1226 static TypeInfo structs.
> 
> Probably possible to do a reasonably good job with a perl script or
> similar. The code patterns to be replaced are reasonably easy to
> identify with a few regexes.

I've attempted to parse all the TypeInfo structs in the tree.
The data I've extracted is available at:
https://gist.github.com/ehabkost/7a398640492f369685c789ffed0f67aa

It turns out 230 of our 1259 TypeInfo variables don't have
instance_size set and don't have their own struct type defined.

We could:
* Make that a supported use case, and add helper macros that don't
  require MyDevice to be defined;
* Make that not supported, and convert those 230 types automatically; or
* Make that not supported, and convert those 230 types manually.

-- 
Eduardo



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

* Re: [PATCH 3/4] crypto: use QOM macros for declaration/definition of secret types
  2020-08-06 18:01       ` Eduardo Habkost
@ 2020-08-07 11:11         ` Daniel P. Berrangé
  2020-08-08  1:38           ` Eduardo Habkost
  0 siblings, 1 reply; 18+ messages in thread
From: Daniel P. Berrangé @ 2020-08-07 11:11 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Peter Maydell, qemu-devel, Markus Armbruster, Paolo Bonzini

On Thu, Aug 06, 2020 at 02:01:54PM -0400, Eduardo Habkost wrote:
> On Fri, Jul 24, 2020 at 10:12:45AM +0100, Daniel P. Berrangé wrote:
> > On Thu, Jul 23, 2020 at 02:50:06PM -0400, Eduardo Habkost wrote:
> > > On Thu, Jul 23, 2020 at 07:14:09PM +0100, Daniel P. Berrangé wrote:
> > > > This introduces the use of the OBJECT_DEFINE and OBJECT_DECLARE macro
> > > > families in the secret types, in order to eliminate boilerplate code.
> > > > 
> > > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > > > ---
> > > >  crypto/secret.c                 | 24 ++++--------------------
> > > >  crypto/secret_common.c          | 32 +++++++++-----------------------
> > > >  crypto/secret_keyring.c         | 28 +++++++++-------------------
> > > >  include/crypto/secret.h         | 11 ++---------
> > > >  include/crypto/secret_common.h  | 13 ++-----------
> > > >  include/crypto/secret_keyring.h | 18 ++----------------
> > > >  6 files changed, 28 insertions(+), 98 deletions(-)
> > > > 
> > > 
> > > Beautiful.
> > > 
> > > I wonder how hard it would be to automate this.  I'm assuming
> > > Coccinelle won't be able to deal with the macro definitions, but
> > > a handwritten conversion script would be really useful for
> > > dealing with our 1226 static TypeInfo structs.
> > 
> > Probably possible to do a reasonably good job with a perl script or
> > similar. The code patterns to be replaced are reasonably easy to
> > identify with a few regexes.
> 
> I've attempted to parse all the TypeInfo structs in the tree.
> The data I've extracted is available at:
> https://gist.github.com/ehabkost/7a398640492f369685c789ffed0f67aa
> 
> It turns out 230 of our 1259 TypeInfo variables don't have
> instance_size set and don't have their own struct type defined.
> 
> We could:
> * Make that a supported use case, and add helper macros that don't
>   require MyDevice to be defined;
> * Make that not supported, and convert those 230 types automatically; or
> * Make that not supported, and convert those 230 types manually.

When we force an instance struct, we also force definition of an
instance init and finalize function.

230 types is probably enough to justify a further macro that allows
the instance struct, init & finalize funtions to be omitted.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



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

* Re: [PATCH 3/4] crypto: use QOM macros for declaration/definition of secret types
  2020-08-07 11:11         ` Daniel P. Berrangé
@ 2020-08-08  1:38           ` Eduardo Habkost
  0 siblings, 0 replies; 18+ messages in thread
From: Eduardo Habkost @ 2020-08-08  1:38 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Peter Maydell, qemu-devel, Markus Armbruster, Paolo Bonzini

On Fri, Aug 07, 2020 at 12:11:48PM +0100, Daniel P. Berrangé wrote:
> On Thu, Aug 06, 2020 at 02:01:54PM -0400, Eduardo Habkost wrote:
> > On Fri, Jul 24, 2020 at 10:12:45AM +0100, Daniel P. Berrangé wrote:
> > > On Thu, Jul 23, 2020 at 02:50:06PM -0400, Eduardo Habkost wrote:
> > > > On Thu, Jul 23, 2020 at 07:14:09PM +0100, Daniel P. Berrangé wrote:
> > > > > This introduces the use of the OBJECT_DEFINE and OBJECT_DECLARE macro
> > > > > families in the secret types, in order to eliminate boilerplate code.
> > > > > 
> > > > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > > > > ---
> > > > >  crypto/secret.c                 | 24 ++++--------------------
> > > > >  crypto/secret_common.c          | 32 +++++++++-----------------------
> > > > >  crypto/secret_keyring.c         | 28 +++++++++-------------------
> > > > >  include/crypto/secret.h         | 11 ++---------
> > > > >  include/crypto/secret_common.h  | 13 ++-----------
> > > > >  include/crypto/secret_keyring.h | 18 ++----------------
> > > > >  6 files changed, 28 insertions(+), 98 deletions(-)
> > > > > 
> > > > 
> > > > Beautiful.
> > > > 
> > > > I wonder how hard it would be to automate this.  I'm assuming
> > > > Coccinelle won't be able to deal with the macro definitions, but
> > > > a handwritten conversion script would be really useful for
> > > > dealing with our 1226 static TypeInfo structs.
> > > 
> > > Probably possible to do a reasonably good job with a perl script or
> > > similar. The code patterns to be replaced are reasonably easy to
> > > identify with a few regexes.
> > 
> > I've attempted to parse all the TypeInfo structs in the tree.
> > The data I've extracted is available at:
> > https://gist.github.com/ehabkost/7a398640492f369685c789ffed0f67aa
> > 
> > It turns out 230 of our 1259 TypeInfo variables don't have
> > instance_size set and don't have their own struct type defined.
> > 
> > We could:
> > * Make that a supported use case, and add helper macros that don't
> >   require MyDevice to be defined;
> > * Make that not supported, and convert those 230 types automatically; or
> > * Make that not supported, and convert those 230 types manually.
> 
> When we force an instance struct, we also force definition of an
> instance init and finalize function.
> 
> 230 types is probably enough to justify a further macro that allows
> the instance struct, init & finalize funtions to be omitted.

Status update: the TypeInfo parser evolved to become a converter
able to replace the type checking macros (automatic conversion of
TypeInfo declarations will be done soon).

https://github.com/ehabkost/qemu-hacks/commits/work/qom-macros-autoconvert

Additional obstacles we'll need to address:

- Sometimes the struct typedefs are in a completely different
  file from the type checking macros.  I've worked around this
  problem by introducing macros that will only add the type
  casting functions, but no typedefs.
- There's some usage of const object pointers in the code,
  which breaks the new type cast functions:
  https://travis-ci.org/github/ehabkost/qemu-hacks/jobs/716033062#L1417

We can probably use _Generic to make the type cast functions
const-safe, but I'm sure this will break existing code that
expects the type casts to always return non-const pointers.

-- 
Eduardo



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

* Re: [PATCH 2/4] qom: provide convenient macros for declaring and defining types
  2020-07-23 18:14 ` [PATCH 2/4] qom: provide convenient macros for declaring and defining types Daniel P. Berrangé
  2020-07-23 19:23   ` Eric Blake
@ 2020-08-09  4:21   ` Eduardo Habkost
  2020-08-11 18:22   ` Eduardo Habkost
  2 siblings, 0 replies; 18+ messages in thread
From: Eduardo Habkost @ 2020-08-09  4:21 UTC (permalink / raw)
  To: Daniel P. Berrangé; +Cc: Paolo Bonzini, qemu-devel

There are implicit requirements in the macros that will be a problem in some
existing type definitions:

On Thu, Jul 23, 2020 at 07:14:08PM +0100, Daniel P. Berrangé wrote:
[...]
> +/**
> + * OBJECT_DECLARE_TYPE:
> + * @ModuleObjName: the object name with initial capitalization
> + * @module_obj_name: the object name in lowercase with underscore separators
> + * @MODULE_OBJ_NAME: the object name in uppercase with underscore separators
> + *
> + * This macro is typically used in a header file, and will:
> + *
> + *   - create the typedefs for the object and class structs
> + *   - register the type for use with g_autoptr
> + *   - provide three standard type cast functions
> + *
> + * The object struct and class struct need to be declared manually.
> + */
> +#define OBJECT_DECLARE_TYPE(ModuleObjName, module_obj_name, MODULE_OBJ_NAME) \
> +    typedef struct ModuleObjName ModuleObjName; \
> +    typedef struct ModuleObjName##Class ModuleObjName##Class; \

The ModuleObjName / ModuleObjName##Class pattern will be a problem in the following cases:

hw/arm/mps2-tz.c:107:1: class typedef MPS2TZMachineClass doesn't match instance typedef MPS2TZMachineState
hw/arm/mps2.c:83:1: class typedef MPS2MachineClass doesn't match instance typedef MPS2MachineState
hw/arm/musca.c:89:1: class typedef MuscaMachineClass doesn't match instance typedef MuscaMachineState
hw/arm/raspi.c:53:1: class typedef RaspiMachineClass doesn't match instance typedef RaspiMachineState
hw/arm/spitz.c:59:1: class typedef SpitzMachineClass doesn't match instance typedef SpitzMachineState
hw/arm/vexpress.c:182:1: class typedef VexpressMachineClass doesn't match instance typedef VexpressMachineState
hw/avr/arduino.c:36:1: class typedef ArduinoMachineClass doesn't match instance typedef ArduinoMachineState
hw/block/m25p80.c:462:1: class typedef M25P80Class doesn't match instance typedef Flash
hw/i386/kvm/i8254.c:40:1: class typedef KVMPITClass doesn't match instance typedef KVMPITState
hw/input/adb-kbd.c:33:1: class typedef ADBKeyboardClass doesn't match instance typedef KBDState
hw/input/adb-mouse.c:32:1: class typedef ADBMouseClass doesn't match instance typedef MouseState
hw/intc/arm_gic_kvm.c:34:1: class typedef KVMARMGICClass doesn't match instance typedef GICState
hw/intc/arm_gicv3_its_kvm.c:32:1: class typedef KVMARMITSClass doesn't match instance typedef GICv3ITSState
hw/intc/arm_gicv3_kvm.c:44:1: class typedef KVMARMGICv3Class doesn't match instance typedef GICv3State
hw/misc/tmp421.c:74:1: class typedef TMP421Class doesn't match instance typedef TMP421State
hw/rtc/m48t59-isa.c:39:1: class typedef M48txxISADeviceClass doesn't match instance typedef M48txxISAState
hw/rtc/m48t59.c:49:1: class typedef M48txxSysBusDeviceClass doesn't match instance typedef M48txxSysBusState
hw/rx/rx-gdbsim.c:53:1: class typedef RxGdbSimMachineClass doesn't match instance typedef RxGdbSimMachineState
hw/scsi/scsi-disk.c:56:1: class typedef SCSIDiskClass doesn't match instance typedef SCSIDiskState
hw/virtio/virtio-pci.h:29:1: class typedef VirtioPCIBusClass doesn't match instance typedef VirtioPCIBusState
hw/virtio/virtio-pci.h:101:1: class typedef VirtioPCIClass doesn't match instance typedef VirtIOPCIProxy
include/hw/boards.h:24:1: class typedef MachineClass doesn't match instance typedef MachineState
include/hw/i386/pc.h:124:1: class typedef PCMachineClass doesn't match instance typedef PCMachineState
include/hw/qdev-core.h:15:1: class typedef DeviceClass doesn't match instance typedef DeviceState
include/hw/qdev-core.h:206:1: class typedef BusClass doesn't match instance typedef BusState

> +    \
> +    G_DEFINE_AUTOPTR_CLEANUP_FUNC(ModuleObjName##Class, object_unref) \
> +    \
> +    static inline G_GNUC_UNUSED ModuleObjName##Class * \
> +    MODULE_OBJ_NAME##_GET_CLASS(void *obj) \
> +    { return OBJECT_GET_CLASS(ModuleObjName##Class, obj, \
> +                              TYPE_##MODULE_OBJ_NAME); } \

The TYPE_##MODULE_OBJ_NAME pattern will be a problem in the following cases:

accel/tcg/tcg-all.c:47:1: macro name TCG_STATE doesn't match type name TYPE_TCG_ACCEL
chardev/baum.c:106:1: macro name BAUM_CHARDEV doesn't match type name TYPE_CHARDEV_BRAILLE
chardev/char-parallel.c:57:1: macro name PARALLEL_CHARDEV doesn't match type name TYPE_CHARDEV_PARALLEL
chardev/char-parallel.c:184:1: macro name PARALLEL_CHARDEV doesn't match type name TYPE_CHARDEV_PARALLEL
chardev/char-pty.c:45:1: macro name PTY_CHARDEV doesn't match type name TYPE_CHARDEV_PTY
chardev/char-ringbuf.c:42:1: macro name RINGBUF_CHARDEV doesn't match type name TYPE_CHARDEV_RINGBUF
chardev/char-socket.c:88:1: macro name SOCKET_CHARDEV doesn't match type name TYPE_CHARDEV_SOCKET
chardev/char-udp.c:45:1: macro name UDP_CHARDEV doesn't match type name TYPE_CHARDEV_UDP
chardev/char-win-stdio.c:40:1: macro name WIN_STDIO_CHARDEV doesn't match type name TYPE_CHARDEV_WIN_STDIO
chardev/msmouse.c:46:1: macro name MOUSE_CHARDEV doesn't match type name TYPE_CHARDEV_MSMOUSE
chardev/testdev.c:41:1: macro name TESTDEV_CHARDEV doesn't match type name TYPE_CHARDEV_TESTDEV
chardev/wctablet.c:87:1: macro name WCTABLET_CHARDEV doesn't match type name TYPE_CHARDEV_WCTABLET
hw/audio/intel-hda.c:205:1: macro name INTEL_HDA doesn't match type name TYPE_INTEL_HDA_GENERIC
hw/char/etraxfs_ser.c:52:1: macro name ETRAX_SERIAL doesn't match type name TYPE_ETRAX_FS_SERIAL
hw/char/virtio-console.c:24:1: macro name VIRTIO_CONSOLE doesn't match type name TYPE_VIRTIO_CONSOLE_SERIAL_PORT
hw/display/ramfb-standalone.c:8:1: macro name RAMFB doesn't match type name TYPE_RAMFB_DEVICE
hw/i386/kvm/i8254.c:40:1: macro name KVM_PIT doesn't match type name TYPE_KVM_I8254
hw/i386/kvm/i8254.c:42:1: macro name KVM_PIT_CLASS doesn't match type name TYPE_KVM_I8254
hw/i386/kvm/i8254.c:44:1: macro name KVM_PIT_GET_CLASS doesn't match type name TYPE_KVM_I8254
hw/i386/kvm/i8259.c:22:1: macro name KVM_PIC_CLASS doesn't match type name TYPE_KVM_I8259
hw/i386/kvm/i8259.c:24:1: macro name KVM_PIC_GET_CLASS doesn't match type name TYPE_KVM_I8259
hw/intc/i8259.c:40:1: macro name PIC_CLASS doesn't match type name TYPE_I8259
hw/intc/i8259.c:41:1: macro name PIC_GET_CLASS doesn't match type name TYPE_I8259
hw/mips/boston.c:46:1: macro name BOSTON doesn't match type name TYPE_MIPS_BOSTON
hw/misc/edu.c:36:1: macro name EDU doesn't match type name TYPE_PCI_EDU_DEVICE
hw/misc/pvpanic.c:31:1: macro name ISA_PVPANIC_DEVICE doesn't match type name TYPE_PVPANIC
hw/net/can/can_kvaser_pci.c:48:1: macro name KVASER_PCI_DEV doesn't match type name TYPE_CAN_PCI_DEV
hw/net/can/can_mioe3680_pci.c:44:1: macro name MIOe3680_PCI_DEV doesn't match type name TYPE_CAN_PCI_DEV
hw/net/can/can_pcm3680_pci.c:44:1: macro name PCM3680i_PCI_DEV doesn't match type name TYPE_CAN_PCI_DEV
hw/net/e1000.c:150:1: macro name E1000 doesn't match type name TYPE_E1000_BASE
hw/net/e1000.c:153:1: macro name E1000_DEVICE_CLASS doesn't match type name TYPE_E1000_BASE
hw/net/e1000.c:156:1: macro name E1000_DEVICE_GET_CLASS doesn't match type name TYPE_E1000_BASE
hw/net/vmxnet3.c:135:1: macro name VMXNET3_DEVICE_CLASS doesn't match type name TYPE_VMXNET3
hw/net/vmxnet3.c:138:1: macro name VMXNET3_DEVICE_GET_CLASS doesn't match type name TYPE_VMXNET3
hw/pci-bridge/pci_expander_bridge.c:41:1: macro name PXB_DEV doesn't match type name TYPE_PXB_DEVICE
hw/pci-bridge/pci_expander_bridge.c:44:1: macro name PXB_PCIE_DEV doesn't match type name TYPE_PXB_PCIE_DEVICE
hw/pci-host/versatile.c:159:1: macro name PCI_VPB doesn't match type name TYPE_VERSATILE_PCI
hw/pci-host/versatile.c:163:1: macro name PCI_VPB_HOST doesn't match type name TYPE_VERSATILE_PCIHOST
hw/ppc/rs6000_mc.c:31:1: macro name RS6000MC_DEVICE doesn't match type name TYPE_RS6000MC
hw/scsi/esp-pci.c:38:1: macro name PCI_ESP doesn't match type name TYPE_AM53C974_DEVICE
hw/scsi/esp-pci.c:417:1: macro name DC390 doesn't match type name TYPE_DC390_DEVICE
hw/scsi/megasas.c:133:1: macro name MEGASAS doesn't match type name TYPE_MEGASAS_BASE
hw/scsi/megasas.c:136:1: macro name MEGASAS_DEVICE_CLASS doesn't match type name TYPE_MEGASAS_BASE
hw/scsi/megasas.c:139:1: macro name MEGASAS_DEVICE_GET_CLASS doesn't match type name TYPE_MEGASAS_BASE
hw/scsi/mptsas.c:46:1: macro name MPT_SAS doesn't match type name TYPE_MPTSAS1068
hw/scsi/vmw_pvscsi.c:66:1: macro name PVSCSI_DEVICE_CLASS doesn't match type name TYPE_PVSCSI
hw/scsi/vmw_pvscsi.c:69:1: macro name PVSCSI_DEVICE_GET_CLASS doesn't match type name TYPE_PVSCSI
hw/sparc/sun4m.c:777:1: macro name SUN4M_RAM doesn't match type name TYPE_SUN4M_MEMORY
hw/sparc64/sun4u.c:490:1: macro name SUN4U_RAM doesn't match type name TYPE_SUN4U_MEMORY
hw/timer/etraxfs_timer.c:51:1: macro name ETRAX_TIMER doesn't match type name TYPE_ETRAX_FS_TIMER
hw/timer/i8254.c:38:1: macro name PIT_CLASS doesn't match type name TYPE_I8254
hw/timer/i8254.c:40:1: macro name PIT_GET_CLASS doesn't match type name TYPE_I8254
hw/tpm/tpm_crb.c:47:1: macro name CRB doesn't match type name TYPE_TPM_CRB
hw/tpm/tpm_spapr.c:31:1: macro name VIO_SPAPR_VTPM doesn't match type name TYPE_TPM_SPAPR
hw/usb/ccid-card-emulated.c:48:1: macro name EMULATED_CCID_CARD doesn't match type name TYPE_EMULATED_CCID
hw/usb/ccid-card-passthru.c:67:1: macro name PASSTHRU_CCID_CARD doesn't match type name TYPE_CCID_PASSTHRU
hw/usb/dev-serial.c:114:1: macro name USB_SERIAL_DEV doesn't match type name TYPE_USB_SERIAL
hw/usb/dev-smartcard-reader.c:63:1: macro name USB_CCID_DEV doesn't match type name CCID_DEV_NAME
hw/usb/dev-storage.c:71:1: macro name USB_STORAGE_DEV doesn't match type name TYPE_USB_STORAGE
hw/usb/redirect.c:147:1: macro name USB_REDIRECT doesn't match type name TYPE_USB_REDIR
hw/usb/tusb6010.c:33:1: macro name TUSB doesn't match type name TYPE_TUSB6010
hw/vfio/ap.c:38:1: macro name VFIO_AP_DEVICE doesn't match type name VFIO_AP_DEVICE_TYPE
hw/vfio/pci.c:46:1: macro name PCI_VFIO doesn't match type name TYPE_VFIO_PCI
net/filter-rewriter.c:25:1: macro name FILTER_COLO_REWRITER doesn't match type name TYPE_FILTER_REWRITER
tests/check-qom-proplist.c:35:1: macro name DUMMY_OBJECT doesn't match type name TYPE_DUMMY
tests/test-qdev-global-props.c:34:1: macro name STATIC_TYPE doesn't match type name TYPE_STATIC_PROPS
tests/test-qdev-global-props.c:130:1: macro name DYNAMIC_TYPE doesn't match type name TYPE_DYNAMIC_PROPS
ui/console.c:1091:1: macro name VC_CHARDEV doesn't match type name TYPE_CHARDEV_VC
ui/gtk.c:181:1: macro name VC_CHARDEV doesn't match type name TYPE_CHARDEV_VC
ui/spice-app.c:48:1: macro name VC_CHARDEV doesn't match type name TYPE_CHARDEV_VC



> +    \
> +    static inline G_GNUC_UNUSED ModuleObjName##Class * \
> +    MODULE_OBJ_NAME##_CLASS(void *klass) \
> +    { return OBJECT_CLASS_CHECK(ModuleObjName##Class, klass, \
> +                                TYPE_##MODULE_OBJ_NAME); } \
> +    \
> +    static inline G_GNUC_UNUSED ModuleObjName * \
> +    MODULE_OBJ_NAME(void *obj) \
> +    { return OBJECT_CHECK(ModuleObjName, obj, \
> +                          TYPE_##MODULE_OBJ_NAME); }
> +
[...]

-- 
Eduardo



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

* Re: [PATCH 2/4] qom: provide convenient macros for declaring and defining types
  2020-07-23 18:14 ` [PATCH 2/4] qom: provide convenient macros for declaring and defining types Daniel P. Berrangé
  2020-07-23 19:23   ` Eric Blake
  2020-08-09  4:21   ` Eduardo Habkost
@ 2020-08-11 18:22   ` Eduardo Habkost
  2 siblings, 0 replies; 18+ messages in thread
From: Eduardo Habkost @ 2020-08-11 18:22 UTC (permalink / raw)
  To: Daniel P. Berrangé; +Cc: Paolo Bonzini, qemu-devel

I've just noticed this:

On Thu, Jul 23, 2020 at 07:14:08PM +0100, Daniel P. Berrangé wrote:
> When creating new QOM types, there is a lot of boilerplate code that
> must be repeated using a standard pattern. This is tedious to write
> and liable to suffer from subtle inconsistencies. Thus it would
> benefit from some simple automation.
> 
> QOM was loosely inspired by GLib's GObject, and indeed GObject suffers
> from the same burden of boilerplate code, but has long provided a set of
> macros to eliminate this burden in the source implementation. More
> recently it has also provided a set of macros to eliminate this burden
> in the header declaration.
> 
> In GLib there are the G_DECLARE_* and G_DEFINE_* family of macros
> for the header declaration and source implementation respectively:
> 
>   https://developer.gnome.org/gobject/stable/chapter-gobject.html
>   https://developer.gnome.org/gobject/stable/howto-gobject.html
> 
> This patch takes inspiration from GObject to provide the equivalent
> functionality for QOM.
> 
> In the header file, instead of:
> 
>     typedef struct MyDevice MyDevice;
>     typedef struct MyDeviceClass MyDeviceClass;
> 
>     G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDeviceClass, object_unref)

Isn't this supposed to be
  G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDevice, object_unref)
?

> 
>     #define MY_DEVICE_GET_CLASS(void *obj) \
>             OBJECT_GET_CLASS(MyDeviceClass, obj, TYPE_MY_DEVICE)
>     #define MY_DEVICE_CLASS(void *klass) \
>             OBJECT_CLASS_CHECK(MyDeviceClass, klass, TYPE_MY_DEVICE)
>     #define MY_DEVICE(void *obj)
>             OBJECT_CHECK(MyDevice, obj, TYPE_MY_DEVICE)
> 
>     struct MyDeviceClass {
>         DeviceClass parent_class;
>     };
> 
> We now have
> 
>     OBJECT_DECLARE_SIMPLE_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE)
> 
> In cases where the class needs some virtual methods, it can be left
> to be implemented manually using
> 
>     OBJECT_DECLARE_TYPE(MyDevice, my_device, MY_DEVICE)
> 
> Note that these macros are including support for g_autoptr() for the
> object types, which is something previously only supported for variables
> declared as the base Object * type.
[...]
> + * <example>
> + *   <title>Expansion from declaring a simple type</title>
> + *   <programlisting>
> + *     typedef struct MyDevice MyDevice;
> + *     typedef struct MyDeviceClass MyDeviceClass;
> + *
> + *     G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDeviceClass, object_unref)
> + *
> + *     #define MY_DEVICE_GET_CLASS(void *obj) \
> + *             OBJECT_GET_CLASS(MyDeviceClass, obj, TYPE_MY_DEVICE)
> + *     #define MY_DEVICE_CLASS(void *klass) \
> + *             OBJECT_CLASS_CHECK(MyDeviceClass, klass, TYPE_MY_DEVICE)
> + *     #define MY_DEVICE(void *obj)
> + *             OBJECT_CHECK(MyDevice, obj, TYPE_MY_DEVICE)
> + *
> + *     struct MyDeviceClass {
> + *         DeviceClass parent_class;
> + *     };
> + *   </programlisting>
> + * </example>
[...]
> +#define OBJECT_DECLARE_TYPE(ModuleObjName, module_obj_name, MODULE_OBJ_NAME) \
> +    typedef struct ModuleObjName ModuleObjName; \
> +    typedef struct ModuleObjName##Class ModuleObjName##Class; \
> +    \
> +    G_DEFINE_AUTOPTR_CLEANUP_FUNC(ModuleObjName##Class, object_unref) \
> +    \
> +    static inline G_GNUC_UNUSED ModuleObjName##Class * \
> +    MODULE_OBJ_NAME##_GET_CLASS(void *obj) \
> +    { return OBJECT_GET_CLASS(ModuleObjName##Class, obj, \
> +                              TYPE_##MODULE_OBJ_NAME); } \
> +    \
> +    static inline G_GNUC_UNUSED ModuleObjName##Class * \
> +    MODULE_OBJ_NAME##_CLASS(void *klass) \
> +    { return OBJECT_CLASS_CHECK(ModuleObjName##Class, klass, \
> +                                TYPE_##MODULE_OBJ_NAME); } \
> +    \
> +    static inline G_GNUC_UNUSED ModuleObjName * \
> +    MODULE_OBJ_NAME(void *obj) \
> +    { return OBJECT_CHECK(ModuleObjName, obj, \
> +                          TYPE_##MODULE_OBJ_NAME); }
> +
[...]

-- 
Eduardo



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

end of thread, other threads:[~2020-08-11 18:23 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-23 18:14 [PATCH 0/4] qom: reduce boilerplate required for declaring and defining objects Daniel P. Berrangé
2020-07-23 18:14 ` [PATCH 1/4] qom: make object_ref/unref use a void * instead of Object * Daniel P. Berrangé
2020-07-23 19:04   ` Eric Blake
2020-07-24  9:06     ` Daniel P. Berrangé
2020-07-23 18:14 ` [PATCH 2/4] qom: provide convenient macros for declaring and defining types Daniel P. Berrangé
2020-07-23 19:23   ` Eric Blake
2020-07-24  9:11     ` Daniel P. Berrangé
2020-08-09  4:21   ` Eduardo Habkost
2020-08-11 18:22   ` Eduardo Habkost
2020-07-23 18:14 ` [PATCH 3/4] crypto: use QOM macros for declaration/definition of secret types Daniel P. Berrangé
2020-07-23 18:50   ` Eduardo Habkost
2020-07-24  9:12     ` Daniel P. Berrangé
2020-08-06 18:01       ` Eduardo Habkost
2020-08-07 11:11         ` Daniel P. Berrangé
2020-08-08  1:38           ` Eduardo Habkost
2020-07-23 19:25   ` Eric Blake
2020-07-23 18:14 ` [PATCH 4/4] crypto: use QOM macros for declaration/definition of TLS creds types Daniel P. Berrangé
2020-07-23 19:26   ` Eric Blake

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.