From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:37720)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1c2YuS-0005Du-7G
	for qemu-devel@nongnu.org; Fri, 04 Nov 2016 03:27:33 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1c2YuN-0005Ti-A0
	for qemu-devel@nongnu.org; Fri, 04 Nov 2016 03:27:32 -0400
Received: from mx1.redhat.com ([209.132.183.28]:42002)
	by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)
	(Exim 4.71) (envelope-from <armbru@redhat.com>) id 1c2YuN-0005T8-3h
	for qemu-devel@nongnu.org; Fri, 04 Nov 2016 03:27:27 -0400
From: Markus Armbruster <armbru@redhat.com>
References: <1477516358-15039-1-git-send-email-eblake@redhat.com>
	<1477516718-15439-3-git-send-email-eblake@redhat.com>
	<87shr8wm4r.fsf@dusky.pond.sub.org>
	<d39af04c-18fe-f890-3ff6-e3da405b3acf@redhat.com>
Date: Fri, 04 Nov 2016 08:27:23 +0100
In-Reply-To: <d39af04c-18fe-f890-3ff6-e3da405b3acf@redhat.com> (Eric Blake's
	message of "Thu, 3 Nov 2016 12:51:06 -0500")
Message-ID: <87k2cjso10.fsf@dusky.pond.sub.org>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH v2 3/3] tests: Enhance qobject output to
 cover partial visit
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: ptoscano@redhat.com, qemu-devel@nongnu.org, Michael Roth <mdroth@linux.vnet.ibm.com>

Eric Blake <eblake@redhat.com> writes:

> On 11/03/2016 11:42 AM, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> writes:
>> 
>>> Add a test that proves (at least when run under valgrind) that
>>> we are correctly handling allocated memory even when a visit
>>> is aborted in the middle for whatever other reason.
>>>
>>> See commit f24582d "qapi: fix double free in
>>> qmp_output_visitor_cleanup()" for a fix that was lacking
>>> testsuite exposure prior to this patch.
>>>
>>> Signed-off-by: Eric Blake <eblake@redhat.com>
>
>>> +static void test_visitor_out_partial_visit(TestOutputVisitorData *data,
>>> +                                           const void *unused)
>>> +{
>>> +    /* Various checks that a mid-visit abort doesn't leak or double-free. */
>>> +    const char *str = "hi";
>>> +    Error *err = NULL;
>>> +    UserDefAlternate uda = { .type = QTYPE_QDICT,
>>> +                             .u.udfu = { .integer = 1,
>>> +                                         .string = (char *) "bye",
>>> +                                         .enum1 = -1 } };
>
> ^ Not a valid enum value...

Now I see.

>>> +
>>> +    /* Abort in the middle of an alternate. Alternates can't be
>>> +     * virtually visited, so we get to inline the first half of
>>> +     * visit_type_UserDefAlternate(). */
>>> +    visit_start_alternate(data->ov, NULL, (GenericAlternate **)&obj,
>>> +                          sizeof(uda), false, &error_abort);
>>> +    visit_start_struct(data->ov, NULL, NULL, 0, &error_abort);
>>> +    visit_type_UserDefUnionBase_members(data->ov,
>>> +                                        (UserDefUnionBase *)&uda.u.udfu,
>>> +                                        &err);
>>> +    error_free_or_abort(&err);
>> 
>> Why does this fail?
>
> ...so visiting the UnionBase_members gripes loudly.  But I see your
> point that more comments would be helpful.

Would you like to suggest a fixup for me to squash in on commit?