From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41459) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZzK9i-0004PC-CS for qemu-devel@nongnu.org; Thu, 19 Nov 2015 03:01:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZzK9d-0008Hi-Q1 for qemu-devel@nongnu.org; Thu, 19 Nov 2015 03:01:22 -0500 Received: from mx1.redhat.com ([209.132.183.28]:47796) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZzK9d-0008HY-KC for qemu-devel@nongnu.org; Thu, 19 Nov 2015 03:01:17 -0500 From: Markus Armbruster References: <1447836791-369-1-git-send-email-eblake@redhat.com> <1447836791-369-32-git-send-email-eblake@redhat.com> <874mgjjgbw.fsf@blackfin.pond.sub.org> <564CDAAC.3030200@redhat.com> Date: Thu, 19 Nov 2015 09:01:14 +0100 In-Reply-To: <564CDAAC.3030200@redhat.com> (Eric Blake's message of "Wed, 18 Nov 2015 13:08:12 -0700") Message-ID: <87wptee7ud.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v12 31/36] qapi: Simplify visiting of alternate types List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-devel@nongnu.org, Michael Roth Eric Blake writes: > On 11/18/2015 11:46 AM, Markus Armbruster wrote: >> Eric Blake writes: >> >>> Previously, working with alternates required two lookup arrays >>> and some indirection: for type Foo, we created Foo_qtypes[] >>> which maps each qtype to a value of the generated FooKind enum, >>> then look up that value in FooKind_lookup[] like we do for other >>> union types. >>> >>> This has a couple of subtle bugs. First, the generator was >>> creating a call with a parameter '(int *) &(*obj)->type' where >>> type is an enum type; this is unsafe if the compiler chooses >>> to store the enum type in a different size than int, where >>> assigning through the wrong size pointer can corrupt data or >>> cause a SIGBUS. [We still have the casting bug for our enum >>> visitors, but that's a topic for a different patch.] >> >> I'm not sure I get the last sentence. > > I was referring to our casts of enum types to int* inside visit_type_Enum(): > https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg03408.html > > If you have a better wording for it, or want to drop the parenthetical > altogether, I'm fine. What about this: replace the parenthetical with Related bug, not fixed in this patch: qapi-visit.py's gen_visit_enum() generates a cast of its enum * argument to int *. Marked FIXME. and squash in diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index e8b53b3..4797d6e 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -172,6 +172,7 @@ out: def gen_visit_enum(name): + # FIXME cast from enum *obj to int * invalidly assumes enum is int return mcgen(''' void visit_type_%(c_name)s(Visitor *v, %(c_name)s *obj, const char *name, Error **errp)