From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NGATL-0006Bs-GR for qemu-devel@nongnu.org; Thu, 03 Dec 2009 07:07:47 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NGATG-00069s-Ek for qemu-devel@nongnu.org; Thu, 03 Dec 2009 07:07:46 -0500 Received: from [199.232.76.173] (port=59550 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NGATG-00069j-6o for qemu-devel@nongnu.org; Thu, 03 Dec 2009 07:07:42 -0500 Received: from mx1.redhat.com ([209.132.183.28]:17834) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NGATF-00078B-K5 for qemu-devel@nongnu.org; Thu, 03 Dec 2009 07:07:41 -0500 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id nB3C7e9G015331 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 3 Dec 2009 07:07:40 -0500 Date: Thu, 3 Dec 2009 14:04:41 +0200 From: "Michael S. Tsirkin" Message-ID: <20091203120441.GD7352@redhat.com> References: <20091202134153.GC18193@redhat.com> <20091202181911.GB3949@redhat.com> <20091202184410.GC3984@redhat.com> <20091203094836.GA6752@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: [Qemu-devel] Re: [PATCH 06/41] virtio: Use DO_UPCAST instead of a cast List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Juan Quintela Cc: qemu-devel@nongnu.org On Thu, Dec 03, 2009 at 12:56:57PM +0100, Juan Quintela wrote: > "Michael S. Tsirkin" wrote: > > On Wed, Dec 02, 2009 at 08:03:22PM +0100, Juan Quintela wrote: > >> "Michael S. Tsirkin" wrote: > >> > >> > I don't understand. > >> > container_of is just more generic than DO_UPCAST. > >> > So why *ever* use DO_UPCAST? Let's get rid of it. > > .... > > >> See how you create a device of size struct_size, but then you access it > >> with vdev. If vdev is _not_ the 1st element of the struct, you have got > >> corruption. > > > > A cleaner solution IMO would be to have callers allocate the memory > > and pass VirtIODevice * to virtio_common_init. > > Been there, asked for that. Basically qdev + passing initialized memory > = nono It does not have to be initialized. > >> DO_UPCAST() prevent you for having that error. > > > > > > If we want to assert specific structure layout, this > > should be a compile-time check. There's no > > reason to do this check every time at a random place where > > DO_UPCAST is called. > > See DO_UPCAST() definition :) > > > It is a compile time check. It just do cpp magic to be sure that things > are right. DO_UPCAST() == (cast *) with some typechecking. Yes, but it gives the error on the wrong place. So it is only good as long as you do not change the code. In the example you give with virtio_init_common, there's no UPCAST to document the layout assumption *in the place where assumption is made*. On the other hand, DO_UPCAST is scattered all over the code in places which could work fine without any assumptions. Let's assume you want to change layout. You find all places with DO_UPCAST, fix them, and it will compile-but not work. Let's assume you change all code that makes layout assumptions to not make them: DO_UPCAST will still be hang around in other code. > >> container_of() would have leave you go around, and have a memory > >> corruption not easy to fix. > >> > >> DO_UPCAST() macro was created just to avoid this kind of errors. > >> > >> Later, Juan.