From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44975) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XSVaN-0005vO-9N for qemu-devel@nongnu.org; Fri, 12 Sep 2014 14:28:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XSVaE-0001Vq-Sa for qemu-devel@nongnu.org; Fri, 12 Sep 2014 14:28:43 -0400 Received: from e36.co.us.ibm.com ([32.97.110.154]:59281) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XSVaE-0001Ve-LN for qemu-devel@nongnu.org; Fri, 12 Sep 2014 14:28:34 -0400 Received: from /spool/local by e36.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 12 Sep 2014 12:28:33 -0600 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Roth In-Reply-To: <54131F7A.3000809@redhat.com> References: <1410477659-9163-1-git-send-email-mdroth@linux.vnet.ibm.com> <1410477659-9163-2-git-send-email-mdroth@linux.vnet.ibm.com> <5412C828.2070004@redhat.com> <20140912153447.19243.81596@loki> <541313C5.40103@redhat.com> <20140912161720.19243.93082@loki> <54131F7A.3000809@redhat.com> Message-ID: <20140912182827.19243.69082@loki> Date: Fri, 12 Sep 2014 13:28:27 -0500 Subject: Re: [Qemu-devel] [PATCH 1/3] qapi: add visit_start_union and visit_end_union List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org Cc: armbru@redhat.com, famz@redhat.com, qemu-stable@nongnu.org, lcapitulino@redhat.com Quoting Paolo Bonzini (2014-09-12 11:29:46) > Il 12/09/2014 18:17, Michael Roth ha scritto: > > Quoting Paolo Bonzini (2014-09-12 10:39:49) > >> Il 12/09/2014 17:34, Michael Roth ha scritto: > >>> > >>> { 'union': 'UserDefUnion', > >>> 'base': 'UserDefZero', > >>> 'data': { 'a' : 'int', 'b' : 'UserDefB' } } > >>> > >>> If UserDefUnion.a is 0, UserDefUnion.data will cast it to a NULL valu= e and > >>> cause the output visitor to bail, when really it should just be left = to > >>> continue on serializing the integer. > >> > >> In the case of dealloc, that'd be okay because the dealloc visit would > >> do nothing for KIND_A, right? > > = > > Yup that should be fine for the dealloc visitor. With this series we ne= ver > > actually visit the int in this case though due to this quirk. But that's > > okay because it's not an allocated type and the dealloc visitor doesn't= need > > to do anything anyway. (It's a bit wonky, but since that reliance on > > implementation details now lives in the visitor implementation of > > visit_start_union it's reasonably contained at least) > > = > > But if we're looking at extending visit_start_union for use in somethin= g like > > an output visitor, this would need to be addressed some other way, since > > skipping scalar fields because they're 0 is a bug there. > = > I guess it would be something like > = > has_data =3D (kind < KIND_MAX) && (is_scalar[kind] || !!data) > = > That could be done in qapi-visit.py if we were so inclined... Yah that should be everything we'd need, but we'd need to make other changes similar to what Fam originally proposed to ensure kind < KIND_MAX implies t= hat kind has actually been initialized. Or, we'd need to make all enums start a= t 1, and reserve 0 for INVALID. Not aware if any option except those 2 atm. However, we could still actually implement what you proposed for has_data a= s is, and make use of the fact that even if kind happens to be invalid/uninitiali= zed, we still won't attempt to visit/dereference the value in an output visitor = (if they implement visit_start_union) if that value is NULL or scalar(0). So, it makes at least one case safer. It wouldn't stop us for doing somethi= ng like serializing a char* as an integer or something along that line though,= so it's somewhat of a false assurance unless we do something to validate .kind. > = > Paolo