[libvirt] PATCH: Switch all remaining code to memory alloc APIs

[Daniel P. Berrange]

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