From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45738) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c2L62-0000rP-VT for qemu-devel@nongnu.org; Thu, 03 Nov 2016 12:42:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c2L5z-0005US-Rs for qemu-devel@nongnu.org; Thu, 03 Nov 2016 12:42:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37506) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c2L5z-0005UM-KT for qemu-devel@nongnu.org; Thu, 03 Nov 2016 12:42:31 -0400 From: Markus Armbruster References: <1477516358-15039-1-git-send-email-eblake@redhat.com> <1477516718-15439-3-git-send-email-eblake@redhat.com> Date: Thu, 03 Nov 2016 17:42:28 +0100 In-Reply-To: <1477516718-15439-3-git-send-email-eblake@redhat.com> (Eric Blake's message of "Wed, 26 Oct 2016 16:18:38 -0500") Message-ID: <87shr8wm4r.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: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-devel@nongnu.org, Michael Roth , ptoscano@redhat.com Eric Blake 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 > > --- > v2: rebase (and hence retitle), add list & alternate coverage > --- > tests/test-qobject-output-visitor.c | 39 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/tests/test-qobject-output-visitor.c b/tests/test-qobject-output-visitor.c > index c2e0f43..fdae0d5 100644 > --- a/tests/test-qobject-output-visitor.c > +++ b/tests/test-qobject-output-visitor.c > @@ -254,6 +254,43 @@ static void test_visitor_out_struct_errors(TestOutputVisitorData *data, > } > > > +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 } }; > + UserDefAlternate *obj = &uda; > + > + /* Abort within a nested object with no data members */ > + visit_start_struct(data->ov, NULL, NULL, 0, &error_abort); > + visit_start_struct(data->ov, "nested", NULL, 0, &error_abort); > + visitor_reset(data); > + > + /* Abort in the middle of a list of strings */ > + visit_start_list(data->ov, "list", NULL, 0, &error_abort); > + visit_type_str(data->ov, NULL, (char **)&str, &error_abort); > + visit_type_str(data->ov, NULL, (char **)&str, &error_abort); > + visitor_reset(data); > + > + /* 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? > + visitor_reset(data); > +} > + > + > static void test_visitor_out_list(TestOutputVisitorData *data, > const void *unused) > { > @@ -817,6 +854,8 @@ int main(int argc, char **argv) > &out_visitor_data, test_visitor_out_struct_nested); > output_visitor_test_add("/visitor/output/struct-errors", > &out_visitor_data, test_visitor_out_struct_errors); > + output_visitor_test_add("/visitor/output/partial-visit", > + &out_visitor_data, test_visitor_out_partial_visit); > output_visitor_test_add("/visitor/output/list", > &out_visitor_data, test_visitor_out_list); > output_visitor_test_add("/visitor/output/any",