From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45253) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zz7BG-0001cC-6C for qemu-devel@nongnu.org; Wed, 18 Nov 2015 13:10:07 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zz7BC-0006i9-7M for qemu-devel@nongnu.org; Wed, 18 Nov 2015 13:10:06 -0500 From: Markus Armbruster References: <1447836791-369-1-git-send-email-eblake@redhat.com> <1447836791-369-23-git-send-email-eblake@redhat.com> <87610zpi2b.fsf@blackfin.pond.sub.org> <564CAEB4.1040309@redhat.com> Date: Wed, 18 Nov 2015 19:09:54 +0100 In-Reply-To: <564CAEB4.1040309@redhat.com> (Eric Blake's message of "Wed, 18 Nov 2015 10:00:36 -0700") Message-ID: <87vb8zji19.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v12 22/36] qapi: Don't let implicit enum MAX member collide List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Kevin Wolf , Peter Maydell , Alberto Garcia , Eduardo Habkost , "open list:blkdebug" , "Michael S. Tsirkin" , Jason Wang , Juan Quintela , qemu-devel@nongnu.org, Michael Roth , Andreas =?utf-8?Q?F=C3=A4rber?= , Gerd Hoffmann , Stefan Hajnoczi , Paolo Bonzini , Amit Shah , "Denis V. Lunev" , Luiz Capitulino , Richard Henderson Eric Blake writes: > On 11/18/2015 06:12 AM, Markus Armbruster wrote: >> Eric Blake 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 >> >> 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.