From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:53776)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1aORMh-0006Xf-7F
	for qemu-devel@nongnu.org; Wed, 27 Jan 2016 09:46:36 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1aORMe-00075W-1p
	for qemu-devel@nongnu.org; Wed, 27 Jan 2016 09:46:35 -0500
Received: from mx1.redhat.com ([209.132.183.28]:58460)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1aORMd-00075R-So
	for qemu-devel@nongnu.org; Wed, 27 Jan 2016 09:46:31 -0500
From: Markus Armbruster <armbru@redhat.com>
References: <1453219845-30939-27-git-send-email-eblake@redhat.com>
	<1453902888-20457-1-git-send-email-armbru@redhat.com>
	<1453902888-20457-3-git-send-email-armbru@redhat.com>
	<56A8CE12.6020508@redhat.com>
Date: Wed, 27 Jan 2016 15:46:28 +0100
In-Reply-To: <56A8CE12.6020508@redhat.com> (Eric Blake's message of "Wed, 27
	Jan 2016 07:02:58 -0700")
Message-ID: <874mdzhygr.fsf@blackfin.pond.sub.org>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH 2/3] qapi-visit: Clean up code generated
	around visit_end_union()
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, mdroth@linux.vnet.ibm.com

Eric Blake <eblake@redhat.com> writes:

> On 01/27/2016 06:54 AM, Markus Armbruster wrote:
>> The generated code can call visit_end_union() without having called
>> visit_start_union().  Example:
>> 
>> 	if (!*obj) {
>> 	    goto out_obj;
>> 	}
>> 	visit_type_BlockdevOptions_fields(v, obj, &err);
>> 	if (err) {
>> 	    goto out_obj; // if we go from here...
>> 	}
>> 	if (!visit_start_union(v, !!(*obj)->u.data, &err) || err) {
>> 	    goto out_obj;
>> 	}
>> 	switch ((*obj)->driver) {
>>     [...]
>> 	}
>>     out_obj:
>>         // ... then *obj is true, and ...
>> 	error_propagate(errp, err);
>> 	err = NULL;
>> 	if (*obj) {
>> 	    // we end up here
>> 	    visit_end_union(v, !!(*obj)->u.data, &err);
>> 	}
>> 	error_propagate(errp, err);

Tabs crept into my commit message, oops.

>> 
>> Harmless only because no visitor implements end_union().  Clean it up
>> anyway.
>
> I plan on deleting visit_end_union() anyways (and visit_start_union);
> see 32/37, plus the FIXME comments added in 21/37.  Maybe it's easier to
> just delete this incorrect and unused callback earlier in the series,
> using your commit message as additional rationale why it is worthless,
> and leaving only visit_start_union() cleanups for 32/37.

I could've tried to move PATCH 32 before the unification, but that
would've been work, so I went with this straightforward fix instead.
Sure, it fixes something that'll go away soon, but I think the churn is
quite tolerable: 1 file changed, 2 insertions(+), 4 deletions(-).

>> Messed up since we have visit_end_union (commit cee2ded).
>
> Indeed.