Re: [Qemu-devel] [PATCH v7 07/14] qapi: Move union tag quirks into subclass

[Eric Blake]

On 10/08/2015 06:25 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
> 
>> Right now, simple unions have a quirk of using 'kind' in the C
>> struct to match the QMP wire name 'type'.  This has resulted in
>> messy clients each doing special cases.  While we plan to
>> eventually rename things to match, it is better in the meantime
>> to consolidate the quirks into a special subclass, by adding a
>> new member.c_name() function.  This will also make it easier
>> for reworking how alternate types are laid out in a future
>> patch.  Use the new c_name() function where possible.
>>
>> No change to generated code.
>>
>> Signed-off-by: Eric Blake <address@hidden>
>>
>> ---
>> v7: new patch, but borrows idea of subclass from v6 10/12, as
>> well as c_name() from 7/12
>> ---
>>  scripts/qapi-commands.py |  8 ++++----
>>  scripts/qapi-types.py    | 12 +++++-------
>>  scripts/qapi-visit.py    | 17 +++++------------
>>  scripts/qapi.py          | 15 +++++++++++++--
>>  4 files changed, 27 insertions(+), 25 deletions(-)
> 
> My immediate reaction to the subclass idea was "instead of encapsulating
> the flaw more nicely, why not fix it?"  So gave that a try, see my other
> reply.

I had already done the same sort of fix, but it was just sitting later
in my series where you hadn't reached reviewing yet.

> 
> That said, the diffstat shows the subclass idea doesn't take much code.
> May make sense if we feel we shouldn't fix the flaw now.

I also like the subclass idea because it makes simplifying alternates
easier (see my just-posted subset C).

But it sounds like getting rid of the 'type'/'kind' mismatch sooner
rather than later seems like the direction we should be heading.

If I need to spin a v8 of this series, I'll certainly include that
conversion (whether from mine, yours, or a combination of the two).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature