On Mon, Apr 28, 2008 at 01:05:45PM +0100, Daniel P. Berrange wrote:
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.
clearly i wrote that before my morning coffee had really precolated up
to the brain, thanks for decrypting :-\
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.
Hum, right, that's the only drawback I could find to the proposal. Overall
it really makes sense. The only thing is that it makes slightly harder for
newbies to contribute to the project because they need to learn a bit, but
libvirt is maturing now and that should not be an issue at all.
Actually we should start compiling more complete code rules for the project
The current HACKING is limited to formatting :-) , we probably need
a Documentation/Coding section in the site 9or that could be done in the Wiki)
Daniel
--
Red Hat Virtualization group
http://redhat.com/virtualization/
Daniel Veillard | virtualization library
http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit
http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine
http://rpmfind.net/