Re: [PATCH v7 17/65] qom: add object_{class_}property_add_qapi_enum

[Markus Armbruster <armbru@redhat.com>]

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Hi
>
> On Fri, Oct 30, 2026 at 2:02 PM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>>
>> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> > ---
>> >  include/qom/object.h | 74 ++++++++++++++++++++++++++++++++++++++++++
>> >  qom/object.c         | 90 +++++++++++++++++++++++++++++++++++++++++++++++++---
>> >  2 files changed, 159 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/include/qom/object.h b/include/qom/object.h
>> > index eb61d8842b80..e04be4caed57 100644
>> > --- a/include/qom/object.h
>> > +++ b/include/qom/object.h
>> > @@ -2027,6 +2027,80 @@ ObjectProperty *object_class_property_add_enum(ObjectClass *klass,
>> >                                      int (*get)(Object *, Error **),
>> >                                      void (*set)(Object *, int, Error **));
>> >
>> > +/**
>> > + * struct QapiEnumProp: Descriptor for a property backed by a QAPI enum type
>> > + *
>> > + * Binds a QOM object property to a QAPI enum, providing automatic
>> > + * string<->int conversion through QAPI visitors and optional
>> > + * default-value initialization during object instance init.
>> > + *
>> > + * Use the QAPI_ENUM_PROP() macro to construct instances inline.
>> > + *
>> > + * @name: property name exposed on the QOM object
>> > + * @description: human-readable description (shown in ``-device help``, etc.)
>> > + * @default_value: initial enum value applied via @set during instance init,
>> > + *                 or -1 (the QAPI_ENUM_PROP default) to skip initialization
>> > + * @qapi_type: pointer to the generated QAPITypeInfo for the enum
>> > + *             (provides the string<->int lookup table)
>> > + * @get: getter that returns the current enum value as int, or -1 on error
>> > + * @set: setter that receives the enum value as int; %NULL for read-only props
>> > + */
>> > +typedef struct QapiEnumProp {
>> > +    const char *name;
>> > +    const char *description;
>> > +    const int default_value;
>> > +    const QAPITypeInfo *qapi_type;
>> > +    int (*get)(Object *, Error **);
>> > +    void (*set)(Object *, int, Error **);
>> > +} QapiEnumProp;
>> > +
>> > +#define QAPI_ENUM_PROP(...) ({                                           \
>> > +    static const QapiEnumProp _prop = {                                  \
>> > +        .default_value = -1, __VA_ARGS__                                 \
>> > +    };                                                                   \
>> > +    &_prop; })
>>
>> This is a cute and reasonably well-known macro trick for optional
>> keyword arguments in C.
>>
>> The example [*] below specifies all arguments.  What happens when we
>> omit any?  They default to zero via default initializaion, except for
>> .default_value, which defaults to -1.
>>
>> Trap for developers: some arguments aren't actually optional!  If you
>> forget to specify @name or @qapi_type, the code compiles, but crashes at
>> run time.
>>
>> Not a fan.
>
> Agree (not a fan of C either). We could probably add more
> static/runtime checks. Or clearly state that there is room for
> improvements, or just drop it altogether - either as follow up or with
> the series. I leave that up to you since you are going to respin it.
> thanks!

I dropped it for v8.  Was a bit of a pain, but the patches became
simpler.

[...]