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