Re: [RFC PATCH] include/qom/object.h: New OBJECT_DEFINE_SIMPLE_TYPE{,_WITH_INTEFACES} macros

["Daniel P. Berrangé" <berrange@redhat.com>]

On Mon, Feb 12, 2024 at 05:49:25PM +0000, Peter Maydell wrote:
> We have an OBJECT_DEFINE_TYPE_EXTENDED macro, plus several variations
> on it, which emits the boilerplate for the TypeInfo and ensures it is
> registered with the type system.  However, all the existing macros
> insist that the type being defined has its own FooClass struct, so
> they aren't useful for the common case of a simple leaf class which
> doesn't have any new methods or any other need for its own class
> struct (that is, for the kind of type that OBJECT_DECLARE_SIMPLE_TYPE
> declares).
> 
> Pull the actual implementation of OBJECT_DEFINE_TYPE_EXTENDED out
> into a new DO_OBJECT_DEFINE_TYPE_EXTENDED which parameterizes the
> value we use for the class_size field.  This lets us add a new
> OBJECT_DEFINE_SIMPLE_TYPE which does the same job as the various
> existing OBJECT_DEFINE_*_TYPE_* family macros for this kind of simple
> type, and the variant OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES for
> when the type will implement some interfaces.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I've marked this RFC largely because this patch doesn't include
> any uses of the new macros. I wanted them for a series I'm currently
> working on, but I wanted to send this out early so it could have an
> extended review period and a bit more visibility than if I stuck
> it inside an 8 patch series about some other topic. (In fact, this
> is the second time I looked at the OBJECT_DEFINE_* macros and found
> they weren't suitable for the common case kind of type. The first
> time around I went back to writing the type out the old fashioned
> way, but this time I figured I'd try improving the macros.)

The macros were intended to simplify QOM boilerplate. So given that
they're not currently addressing a common use case of no-Class objects,
it makes sense to extend the macros as you suggest here.

>  docs/devel/qom.rst   |  34 +++++++++++--
>  include/qom/object.h | 114 +++++++++++++++++++++++++++++++++----------
>  2 files changed, 117 insertions(+), 31 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

> 
> diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst
> index 9918fac7f21..0889ca949c1 100644
> --- a/docs/devel/qom.rst
> +++ b/docs/devel/qom.rst
> @@ -348,12 +348,14 @@ 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:
> +For the simplest case of a leaf class which doesn't need any of its
> +own virtual functions (i.e. which was declared with OBJECT_DECLARE_SIMPLE_TYPE)
> +the OBJECT_DEFINE_SIMPLE_TYPE macro is suitable:
>  
>  .. code-block:: c
>     :caption: Defining a simple type
>  
> -   OBJECT_DEFINE_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE)
> +   OBJECT_DEFINE_SIMPLE_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE)
>  
>  This is equivalent to the following:
>  
> @@ -370,7 +372,6 @@ This is equivalent to the following:
>         .instance_size = sizeof(MyDevice),
>         .instance_init = my_device_init,
>         .instance_finalize = my_device_finalize,
> -       .class_size = sizeof(MyDeviceClass),
>         .class_init = my_device_class_init,
>     };
>  
> @@ -385,13 +386,36 @@ 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 class needs its own virtual methods, or has some other
> +per-class state it needs to store in its own class struct,
> +then you can use the OBJECT_DEFINE_TYPE macro. This does the
> +same thing as OBJECT_DEFINE_SIMPLE_TYPE, but it also sets the
> +class_size of the type to the size of the class struct.
> +
> +.. code-block:: c
> +   :caption: Defining a type which needs a class struct
> +
> +   OBJECT_DEFINE_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE)
> +
>  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.
> +OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES() and
> +OBJECT_DEFINE_TYPE_WITH_INTERFACES() macros can be used instead.
> +These accept an array of interface type names. The difference between
> +them is that the former is for simple leaf classes that don't need
> +a class struct, and the latter is for when you will be defining
> +a class struct.
>  
>  .. code-block:: c
>     :caption: Defining a simple type implementing interfaces
>  
> +   OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(MyDevice, my_device,
> +                                             MY_DEVICE, DEVICE,
> +                                             { TYPE_USER_CREATABLE },
> +                                             { NULL })
> +
> +.. code-block:: c
> +   :caption: Defining a type implementing interfaces
> +
>     OBJECT_DEFINE_TYPE_WITH_INTERFACES(MyDevice, my_device,
>                                        MY_DEVICE, DEVICE,
>                                        { TYPE_USER_CREATABLE },
> diff --git a/include/qom/object.h b/include/qom/object.h
> index afccd24ca7a..f52ab216cdd 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -258,6 +258,51 @@ struct Object
>      DECLARE_INSTANCE_CHECKER(InstanceType, MODULE_OBJ_NAME, TYPE_##MODULE_OBJ_NAME)
>  
>  
> +/**
> + * DO_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
> + * @CLASS_SIZE: size of the type's class
> + * @...: list of initializers for "InterfaceInfo" to declare implemented interfaces
> + *
> + * This is the base macro used to implement all the OBJECT_DEFINE_*
> + * macros. It should never be used directly in a source file.
> + */
> +#define DO_OBJECT_DEFINE_TYPE_EXTENDED(ModuleObjName, module_obj_name, \
> +                                       MODULE_OBJ_NAME, \
> +                                       PARENT_MODULE_OBJ_NAME, \
> +                                       ABSTRACT, CLASS_SIZE, ...) \
> +    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_align = __alignof__(ModuleObjName), \
> +        .instance_init = module_obj_name##_init, \
> +        .instance_finalize = module_obj_name##_finalize, \
> +        .class_size = CLASS_SIZE, \
> +        .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_EXTENDED:
>   * @ModuleObjName: the object name with initial caps
> @@ -284,32 +329,10 @@ struct Object
>  #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_align = __alignof__(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);
> +    DO_OBJECT_DEFINE_TYPE_EXTENDED(ModuleObjName, module_obj_name, \
> +                                   MODULE_OBJ_NAME, PARENT_MODULE_OBJ_NAME, \
> +                                   ABSTRACT, sizeof(ModuleObjName##Class), \
> +                                   __VA_ARGS__)
>  
>  /**
>   * OBJECT_DEFINE_TYPE:
> @@ -368,6 +391,45 @@ struct Object
>                                  MODULE_OBJ_NAME, PARENT_MODULE_OBJ_NAME, \
>                                  true, { NULL })
>  
> +/**
> + * OBJECT_DEFINE_SIMPLE_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
> + *
> + * This is a variant of OBJECT_DEFINE_TYPE_EXTENDED, which is suitable for
> + * the case of a non-abstract type, with interfaces, and with no requirement
> + * for a class struct.
> + */
> +#define OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(ModuleObjName, \
> +                                                  module_obj_name, \
> +                                                  MODULE_OBJ_NAME, \
> +                                                  PARENT_MODULE_OBJ_NAME, ...) \
> +    DO_OBJECT_DEFINE_TYPE_EXTENDED(ModuleObjName, module_obj_name, \
> +                                   MODULE_OBJ_NAME, PARENT_MODULE_OBJ_NAME, \
> +                                   false, 0, __VA_ARGS__)
> +
> +/**
> + * OBJECT_DEFINE_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
> + * @PARENT_MODULE_OBJ_NAME: the parent object name in uppercase with underscore
> + *                          separators
> + *
> + * This is a variant of OBJECT_DEFINE_TYPE_EXTENDED, which is suitable for
> + * the common case of a non-abstract type, without any interfaces, and with
> + * no requirement for a class struct. If you declared your type with
> + * OBJECT_DECLARE_SIMPLE_TYPE then this is probably the right choice for
> + * defining it.
> + */
> +#define OBJECT_DEFINE_SIMPLE_TYPE(ModuleObjName, module_obj_name, \
> +                                  MODULE_OBJ_NAME, PARENT_MODULE_OBJ_NAME) \
> +    OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(ModuleObjName, module_obj_name, \
> +        MODULE_OBJ_NAME, PARENT_MODULE_OBJ_NAME, { NULL })
> +
>  /**
>   * struct TypeInfo:
>   * @name: The name of the type.
> -- 
> 2.34.1
> 

With 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 :|