[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v12 22/36] qapi: Don't let implicit enum MAX mem
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v12 22/36] qapi: Don't let implicit enum MAX member collide |
Date: |
Wed, 18 Nov 2015 19:09:54 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 11/18/2015 06:12 AM, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>>
>>> Now that we guarantee the user doesn't have any enum values
>>> beginning with a single underscore, we can use that for our
>>> own purposes. Renaming ENUM_MAX to ENUM__MAX makes it obvious
>>> that the sentinel is generated.
>>
>> The ENUM__MAX are mildly ugly. If we cared, we could get rid of most or
>> all uses. Right now, I don't.
>>
>> Hmm, perhaps these ENUM__MAX are your motivation for rejecting adjacent
>> [-_] in the previous patch, to catch clashes like (foolishly named) type
>> 'FOO--MAX' with enumeration type 'Foo'. I acknowledge the clash.
>
> Yes, and that was part of my reasoning on 21/36 - but you've convinced
> me to relax that one, so it no longer applies as an argument here.
>
>> However, there are many more clashes we don't attempt to catch,
>> e.g. type 'Foo-lookup' with enumeration type 'Foo'. Perhaps we want to
>> build a real fence later, but right now I don't want us to ram in more
>> scattered fence laths.
>>
>> Might want to say something like "Rejecting enum members named 'MAX' is
>> now useless, and will be dropped in the next patch".
>
> Yes, that helps the commit message. Can you handle it, or do I need to
> send a fixup?
Can do!
>>> This patch was mostly generated by applying a temporary patch:
>>>
>>> |diff --git a/scripts/qapi.py b/scripts/qapi.py
>>> |index e6d014b..b862ec9 100644
>>> |--- a/scripts/qapi.py
>>> |+++ b/scripts/qapi.py
>>> |@@ -1570,6 +1570,7 @@ const char *const %(c_name)s_lookup[] = {
>>> | max_index = c_enum_const(name, 'MAX', prefix)
>>> | ret += mcgen('''
>>> | [%(max_index)s] = NULL,
>>> |+// %(max_index)s
>>> | };
>>> | ''',
>>> | max_index=max_index)
>>>
>>> then running:
>>>
>>> $ cat qapi-{types,event}.c tests/test-qapi-types.c |
>>> sed -n 's,^// \(.*\)MAX,s|\1MAX|\1_MAX|g,p' > list
>>> $ git grep -l _MAX | xargs sed -i -f list
>>>
>>> The only things not generated are the changes in scripts/qapi.py.
>>>
>>> Signed-off-by: Eric Blake <address@hidden>
>>
>> Patch looks good.
>
> The fun part was coming up with the automation, and then making sure it
> was reproducible. :)
May not always be faster than manual, but it sure is a lot more fun!
More reliable, too, in my experience.
[Qemu-devel] [PATCH v12 20/36] blkdebug: Avoid '.' in enum values, Eric Blake, 2015/11/18
[Qemu-devel] [PATCH v12 28/36] qapi: Simplify QObject, Eric Blake, 2015/11/18