Re: [PATCH v4 4/7] nbd: Update qapi to support exporting multiple bitmaps

[Markus Armbruster <armbru@redhat.com>]

Eric Blake <eblake@redhat.com> writes:

> On 10/20/20 3:51 AM, Markus Armbruster wrote:
>
>>> #define QAPI_LIST_ADD(list, element) do { \
>>>     typeof(list) _tmp = g_new(typeof(*(list)), 1); \
>>>     _tmp->value = (element); \
>>>     _tmp->next = (list); \
>>>     (list) = _tmp; \
>>> } while (0)
>>>
>>>
>>> Markus, thoughts on if we should publish this macro,
>> 
>> If it's widely useful.
>> 
>> "git-grep -- '->value ='" matches ~200 times.  A patch converting these
>> to the macro where possible would make a strong case for having the
>> macro.
>> 
>>>                                                      and if so, which
>>> header would be best?
>> 
>> The macro is generic: @list's type may be any of the struct TYPEList we
>> generate for the QAPI type ['TYPE'].
>> 
>> We don't want to generate this macro next to each of these struct
>> definitions.  A non-generic macro would go there, but let's not generate
>> almost a hundred non-generic macros where a single generic one can do
>> the job.
>
> Agreed.
>
>> 
>> The closest we have to a common base is GenericList.  It's in in
>> visitor.h because it's only used by visitors so far.  Adding the macro
>> next it is not so smart, because we don't want non-visitor code to
>> include visitor.h just for this macro.
>
> Also agreed.
>
>> 
>> Perhaps the macro should go into qapi/util.h, and perhaps GenericList
>> and GenericAlternate should move there, too.
>
> GenericList is easy, but GenericAlternate is harder: it would introduce
> a cyclic declaration dependency (generated qapi-builtin-types.h includes
> qapi/util.h for the definition of QEnumLookup, but qapi/util.h declaring
> GenericAlternate would depend on including qapi-builtin-types.h for the
> definition of QType).

You're right.

QType is a built-in QAPI type.  The typedef is generated into
qapi-builtin-types.h.

It needs to be a QAPI type because it's the type of alternates'
(implicit) member @type.

I figure the easiest way to move GenericAlternate (if we want to), is
creating a new header, or rather splitting qapi/util.h into the part
needed by qapi-builtin-types.h and the part that needs
qapi-builtin-types.h.

Doesn't seem to be worth our while now.  We can simply put the macro
into qapi/util.h and call it a day.