Re: [Xen-devel] [PATCH] change tools cpumaps to uint8_t

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

Looks nice. Thanks for doing this.
> @@ -74,27 +69,15 @@
>  {
>      int err = 0;
>      xc_cpupoolinfo_t *info = NULL;
> -    int local_size;
> -    int cpumap_size;
> -    int size;
> +    size_t local_size;
>      DECLARE_SYSCTL;
>      DECLARE_HYPERCALL_BUFFER(uint8_t, local);
>  
> -    local_size = get_cpumap_size(xch);
> -    if (!local_size)
> -    {
> -        PERROR("Could not get number of cpus");
> -        return NULL;
> -    }
> -
> -    local = xc_hypercall_buffer_alloc(xch, local, local_size);
> +    local = xc_cpumap_hypercall_buffer_alloc(xch, local, local_size);
>      if ( local == NULL ) {
>          PERROR("Could not allocate locked memory for xc_cpupool_getinfo");
>          return NULL;
>      }

I'm not that keen on this interface change, I think get_cpumap_size +
xc_hypercall_buffer_alloc was fine as it was.

More importantly I really don't like hiding the taking of the pointer on
the size parameter being hidden inside the macro.

[...]
>      info->cpupool_id = sysctl.u.cpupool_op.cpupool_id;
>      info->sched_id = sysctl.u.cpupool_op.sched_id;
>      info->n_dom = sysctl.u.cpupool_op.n_dom;
> -    bitmap_byte_to_64(info->cpumap, local, local_size * 8);
> +    bcopy(local, info->cpumap, local_size);

bcopy is deprecated, should use memcpy (several times).

Rest looks good, lots of magic numbers disappear ;-)

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel