From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:50976)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1aMBOU-0001i8-Dr
	for qemu-devel@nongnu.org; Thu, 21 Jan 2016 04:19:07 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1aMBOT-0007AJ-7a
	for qemu-devel@nongnu.org; Thu, 21 Jan 2016 04:19:06 -0500
Received: from mx1.redhat.com ([209.132.183.28]:56434)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1aMBOT-00079P-2b
	for qemu-devel@nongnu.org; Thu, 21 Jan 2016 04:19:05 -0500
From: Markus Armbruster <armbru@redhat.com>
References: <1453219845-30939-1-git-send-email-eblake@redhat.com>
	<1453219845-30939-17-git-send-email-eblake@redhat.com>
	<87wpr4krmy.fsf@blackfin.pond.sub.org> <569FF598.1020909@redhat.com>
Date: Thu, 21 Jan 2016 10:19:02 +0100
In-Reply-To: <569FF598.1020909@redhat.com> (Eric Blake's message of "Wed, 20
	Jan 2016 14:01:12 -0700")
Message-ID: <87powvmgs9.fsf@blackfin.pond.sub.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Qemu-devel] [PATCH v9 16/37] qapi: Swap 'name' in visit_*
	callbacks to match public API
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Eric Blake <eblake@redhat.com>
Cc: marcandre.lureau@redhat.com, qemu-devel@nongnu.org, Michael Roth <mdroth@linux.vnet.ibm.com>

Eric Blake <eblake@redhat.com> writes:

> On 01/20/2016 11:55 AM, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> writes:
>>=20
>>> As explained in the previous patches, matching argument order of
>>> 'name, &value' to JSON's "name":value makes sense.  However,
>>> while the last two patches were easy with Coccinelle, I ended up
>>> doing this one all by hand.  Now all the visitor callbacks match
>>> the main interface.
>>>
>>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>> Reviewed-by: Marc-Andr=C3=A9 Lureau <marcandre.lureau@redhat.com>
>>>
>
>>> @@ -30,39 +30,42 @@ struct Visitor
>>>      GenericList *(*next_list)(Visitor *v, GenericList **list, Error **=
errp);
>>>      void (*end_list)(Visitor *v, Error **errp);
>>>
>>> -    void (*type_enum)(Visitor *v, int *obj, const char * const strings=
[],
>>> -                      const char *kind, const char *name, Error **errp=
);
>>> +    void (*type_enum)(Visitor *v, const char *name, int *obj,
>>> +                      const char * const strings[], const char *kind,
>>=20
>> Opportunity to change to 'const char *const'.  I prefer that, because it
>> makes the fact that this is a pointer-* and not a binary operator-*
>> visually obvious.
>>=20
>> Same elsewhere.
>
> Hmm, I probably have churn later in the series.  Will fix.
>
>>>      /* May be NULL; most useful for input visitors. */
>>> -    void (*optional)(Visitor *v, bool *present, const char *name);
>>> +    void (*optional)(Visitor *v, const char *name, bool *present);
>>>
>
>>=20
>> I checked the changes to this file carefully.  Can we rely on the
>> compiler to flag mistakes in the rest of the patch?
>
> C's (intentionally-loose) treatment of 'char *' like 'void *' is a bit
> worrisome, but the fact that we have 'const' on only one of the two
> swapped arguments was indeed enough to make the compiler complain about
> mismatch in parameter types when trying to assign incorrectly-typed
> static functions to the updated struct members.

Okay, I guess that's good enough.