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

[David Turner <digit@google.com>]

[-- Attachment #1: Type: text/plain, Size: 2546 bytes --]

On Tue, Jun 2, 2009 at 10:58 AM, Daniel P. Berrange <berrange@redhat.com>wrote:

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

Agreed, that's the thing I do; and it works really well in practice.


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

[-- Attachment #2: Type: text/html, Size: 3777 bytes --]