Re: [Libvir] RFC: safer memory allocation APIs with compile time checking
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 :|