Re: [libvirt] PATCH: Switch all remaining code to memory alloc APIs
On Fri, May 30, 2008 at 12:13:42PM -0400, Daniel Veillard wrote:
On Fri, May 30, 2008 at 04:01:39PM +0100, Daniel P. Berrange wrote:
> This patch switches all remaining code over to use the memory allocation
> APIs, with exception of virsh which is going to be slightly more complex
>
> It was mostly a straight conversion - there were only a few places which
> weren't checking for failure corecttly - the most notable being sexpr.c.
[...]
> - void *stack, *stacktop;
> + char *stack, *stacktop;
>
> /* allocate a stack for the container */
> - stack = malloc(stacksize);
> - if (!stack) {
> + if (VIR_ALLOC_N(stack, stacksize) < 0) {
hum, interesting side effect ... we must type stuff with the new macros.
Yes, that is correct - the macros use sizeof() to automatically determine
the size of the alloc needed. Although GCC treats sizeof(void) as being
the same as sizeof(char), this is not required by the C standard - it is
technically 'undefined behaviour'. So its safest to just switchto using
a char * for the stack here. An earlier function dealing with stacks in
this same file was already using char * too.
> @@ -1659,8 +1659,7 @@
> /* The allocated memory to cpumap must be 'sizeof(uint64_t)' byte
*
> * for Xen, and also nr_cpus must be 'sizeof(uint64_t) * 8'
*/
> if (maplen < 8) {
> - new = calloc(1, sizeof(uint64_t));
> - if (!new) {
> + if (VIR_ALLOC_N(new, sizeof(uint64_t)) < 0) {
That one worried me, but that works but because we have unsigned char *new
Yeah, I was undecided whether to use sizeof(uint64_t) here, or just hardcode
the value 8 to match the line earlier.
> --- a/src/xmlrpc.c Fri May 30 10:36:42 2008 -0400
> +++ b/src/xmlrpc.c Fri May 30 10:55:44 2008 -0400
> @@ -12,6 +12,7 @@
Hum, i think that's dead code anyway, no ?
Yes, although you might end up using it for VMWare driver if you use the
webservices API ?
> @@ -47,9 +48,8 @@
>
> static xmlRpcValuePtr xmlRpcValueNew(xmlRpcValueType type)
> {
> - xmlRpcValuePtr ret = malloc(sizeof(*ret));
> -
> - if (!ret)
> + xmlRpcValuePtr ret = NULL;
> + if (VIR_ALLOC(ret) < 0)
I don't think we need to set ret to NULL, do we ? VIR_ALLOC always
initialize.
Yes, that is redundant.
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 :|