Re: [RFC PATCH-for-9.0 01/11] qom: Introduce the TypeInfo::can_register() handler

[Thomas Huth <thuth@redhat.com>]

On 22/11/2023 19.30, Philippe Mathieu-Daudé wrote:
> Add a helper to decide at runtime whether a type can
> be registered to the QOM framework or not.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/qom/object.h | 4 ++++
>   qom/object.c         | 3 +++
>   2 files changed, 7 insertions(+)
> 
> diff --git a/include/qom/object.h b/include/qom/object.h
> index afccd24ca7..0d42fe17de 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -372,6 +372,8 @@ struct Object
>    * struct TypeInfo:
>    * @name: The name of the type.
>    * @parent: The name of the parent type.
> + * @can_register: This optional function is called before a type is registered.
> + *   If it exists and returns false, the type is not registered.

The second sentence is quite hard to parse, since it is not quite clear what 
"it" refers to (type or function) and what "registered" means in this 
context (you don't mention type_register() here).

Maybe rather something like:

If set, type_register() uses this function to decide whether the type can be 
registered or not.

?

>    * @instance_size: The size of the object (derivative of #Object).  If
>    *   @instance_size is 0, then the size of the object will be the size of the
>    *   parent object.
> @@ -414,6 +416,8 @@ struct TypeInfo
>       const char *name;
>       const char *parent;
>   
> +    bool (*can_register)(void);
> +
>       size_t instance_size;
>       size_t instance_align;
>       void (*instance_init)(Object *obj);
> diff --git a/qom/object.c b/qom/object.c
> index 95c0dc8285..f09b6b5a92 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -150,6 +150,9 @@ static TypeImpl *type_register_internal(const TypeInfo *info)
>   TypeImpl *type_register(const TypeInfo *info)
>   {
>       assert(info->parent);
> +    if (info->can_register && !info->can_register()) {
> +        return NULL;
> +    }

I have to say that I don't like it too much, since you're trying to fix a 
problem here in common code that clearly belongs to the code in hw/arm/ instead.

What about dropping it, and changing your last patch to replace the 
DEFINE_TYPES(raspi_machine_types) in hw/arm/raspi.c with your own 
implementation of type_register_static_array() that checks the condition there?

  Thomas