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 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list