From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MBPr3-0003MD-09 for qemu-devel@nongnu.org; Tue, 02 Jun 2009 05:00:21 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MBPqx-0003Il-IE for qemu-devel@nongnu.org; Tue, 02 Jun 2009 05:00:19 -0400 Received: from [199.232.76.173] (port=49824 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MBPqx-0003IY-92 for qemu-devel@nongnu.org; Tue, 02 Jun 2009 05:00:15 -0400 Received: from mx1.redhat.com ([66.187.233.31]:53325) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MBPqw-0002uY-OB for qemu-devel@nongnu.org; Tue, 02 Jun 2009 05:00:15 -0400 Date: Tue, 2 Jun 2009 09:58:03 +0100 From: "Daniel P. Berrange" Subject: Re: [Qemu-devel] [PATCH] use qemu_malloc and friends consistently Message-ID: <20090602085803.GB26814@redhat.com> References: <200905290758.11551.jcd@tribudubois.net> <4A1FD6E2.9020006@redhat.com> <200905291407.26757.paul@codesourcery.com> <200905291917.10535.jseward@acm.org> <60cad3f0905291412m670c7a6cw45f9b51f3122ddfb@mail.gmail.com> <4A24D40E.2060704@redhat.com> <4A24D92D.4010704@codemonkey.ws> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4A24D92D.4010704@codemonkey.ws> Reply-To: "Daniel P. Berrange" List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Kevin Wolf , David Turner , qemu-devel@nongnu.org, Jean-Christophe Dubois , Paul Brook , Gerd Hoffmann On Tue, Jun 02, 2009 at 02:47:57AM -0500, Anthony Liguori wrote: > Gerd Hoffmann wrote: > > On 05/29/09 23:12, David Turner wrote: > >> I would even suggest providing helper macros to make the programmer's > >> intent > >> even more clear > >> and less error-prone, as in: > >> > >> #define QEMU_NEW(ptr) (ptr) = > >> qemu_alloc(sizeof(*(ptr))) > >> #define QEMU_NEW_ARRAY(ptr,cnt) (ptr) = > >> qemu_calloc((cnt),sizeof(*(ptr))) > >> #define QEMU_RENEW_ARRAY(ptr,cnt) (ptr) = > >> qemu_realloc((ptr),(cnt),sizeof(*(ptr))) > >> #define QEMU_FREE_ARRAY(ptr) qemu_free(ptr) > > > > The idea to have allocators for arrays (and have them allow > > zero-length arrays) is fine. I wouldn't create two macros for new and > > renew array, you can just use usual realloc semantics (ptr == NULL -> > > alloc). > > > > Also I don't like the syntax that much as you'll have the IMHO > > non-intuitive code like this: > > > > QEMU_NEW_ARRAY(ptr, ...); > > > > instead of > > > > ptr = QEMU_NEW_ARRAY(...); > > > > then. I don't see another easy way to get the automagic sizeof(*ptr) > > stuff done though. > > I've always liked glib's memory functions. It does OOM error handling > and returns NULL when size == 0. If you look at the problems associated with malloc there are many common programmer mistakes, of which failure to check for NULL is just one. IMHO, if you're going to wrap malloc/calloc/etc, then you should aim higher and try to address all the common problems. David's suggestion helps address the problem incorrect sizing too, of which there was an example on this list only last week with VncState/VncDisplasy mixup. Other problems including forgetting to initialize memory, which can be solved by using calloc for everything (though in QEMU's case this may have too much overhead). Double free is another which can be protected against by having the free function also NULL-ify the pointer being freed. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|