[Libvir] RFC: safer memory allocation APIs with compile time checking

[Daniel P. Berrange]

On Mon, Apr 28, 2008 at 03:39:41AM -0400, Daniel Veillard wrote:
> On Sun, Apr 27, 2008 at 08:29:33PM +0100, Daniel P. Berrange wrote:
> > So the primary application usage would be via a set of macros:
> > 
> >     VIR_ALLOC(ptr)
> >     VIR_ALLOC_N(ptr, count)
> >     VIR_REALLOC(ptr)
> 
>   you will need some size here.

Sorry, VIR_REALLOC should not have existed - only REALLOC_N is useful.

> > Note that although we're passing a pointer to a pointer into all these, the
> > first param is still just 'void *' and not 'void **'. This is because 'void *'
> > is defined to hold any type of pointer, and using 'void **' causes gcc to
> > complain bitterly about strict aliasing violations. Internally the impls of
> > these methods will safely cast to 'void **' when deferencing the pointer.
> 
>   One of the problem this introduce is what uses to be a common mistake
> freeing the a non-pointer object which is normally immediately pointed out
> by the compiler whicll be lost because your macro will turn this into 
> a void * to the data, and you will get a runtime problem instead.

That is true, but perhaps it is a worthwhile tradeoff, given that we have
had quite a number of ongoing crash problems in the past with double free()
due to missing NULL assignment. At least if you try to fre a non-pointer
you'll see that error raised the first time the code is run, where as the
double free's have often occurred only in error paths & not seen till post
release.
 
> > All 4 of Alloc/Realloc functions will have __warn_unused_result__ annotation
> > so the caller is forced to check the return value for failure, validated at
> > compile time generating a warning (or fatal compile error with -Werror).
> > 
> > So to wire up the macros to the APIs:
> > 
> >   #define VIR_ALLOC(ptr)            virAlloc(&(ptr), sizeof(*(ptr)))
> >   #define VIR_ALLOC_N(ptr, count)   virAllocN(&(ptr), sizeof(*(ptr)), (count))
> >   #define VIR_REALLOC(ptr)          virRealloc(&(ptr), sizeof(*(ptr)))
> 
>   That i really don't understand. How do you expect to use that realloc
> without passing a new size.

It is a mistake - only REALLOC_N is needed

Dan.
-- 
|: Red Hat, Engineering, Boston   -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 :|