Re: [Xen-devel] [PATCH]libxl: rewrite libxl_cpumap_alloc()

["Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx>]

> -----Original Message-----
> From: Ian Campbell [mailto:Ian.Campbell@xxxxxxxxxx]
> Sent: Wednesday, May 23, 2012 5:51 PM
> On Fri, 2012-05-18 at 03:53 +0100, Zhang, Yang Z wrote:
> > Allow libxl_cpumap_alloc to allocate memory of specific size.
> > Max_cpus equals to zero means allocate the biggest possibly required map.
> 
> Can we get a comment to that effect in the header please.

Sure.

> > -    int max_cpus;
> >      int sz;
> >
> > -    max_cpus = libxl_get_max_cpus(ctx);
> > -    if (max_cpus == 0)
> > -        return ERROR_FAIL;
> > +    if (max_cpus < 0) {
> > +        return ERROR_INVAL;
> > +    } else if (max_cpus == 0) {
> > +        max_cpus = libxl_get_max_cpus(ctx);
> > +        if (max_cpus == 0)
> > +            return ERROR_FAIL;
> > +    }
> 
> I would have written this as
> 
> +    if (max_cpus < 0)
> +        return ERROR_INVAL;
> +    if (max_cpus == 0)
> +        max_cpus = libxl_get_max_cpus(ctx);
> +    if (max_cpus == 0)
> +        return ERROR_FAIL;
> 
> and avoided the nested if and ifelse logic.

Agree.
 
> >
> >      sz = (max_cpus + 7) / 8;
> > -    cpumap->map = calloc(sz, sizeof(*cpumap->map));
> > -    if (!cpumap->map)
> > -        return ERROR_NOMEM;
> > +    cpumap->map = libxl__zalloc(NULL, sz * sizeof(*cpumap->map));
> 
> You might as well use libxl__calloc here.

Agree.

> Should I be expecting a v3 of "libxl: allow to set more than 31 vcpus"
> which uses this patch? If you already sent it then I may have missed it.

Yes, I will send it. 

best regards
yang
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel