Re: [Qemu-devel] [PATCH] use qemu_malloc and friends consistently

["Daniel P. Berrange" <berrange@redhat.com>]

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 :|