Re: [Xen-devel] [PATCH RESEND 02/12] xl: allow for node-wise specification of vcpu pinning

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Dario Faggioli writes ("Re: [PATCH RESEND 02/12] xl: allow for node-wise 
specification of vcpu pinning"):
> On gio, 2013-11-07 at 18:17 +0000, Ian Jackson wrote:
> > > -static int vcpupin_parse(char *cpu, libxl_bitmap *cpumap)
> > > +static int parse_range(const char *str, unsigned long *a, unsigned long 
> > > *b)
> > > +{
> > > +    char *nstr, *endptr;
> > 
> > Missing consts ?
> > 
> Is it? For instance, I'm using endptr like this:
> 
>  *a = *b = strtoul(str, &endptr, 10);
> 
> And the prototype of strtoul is:
>  unsigned long int strtoul(const char *nptr, char **endptr, int base);

Oh, yes, sorry, you're right about endptr.

> So it won't work, not for endptr at least. What I can do is the
> following:
> 
>  const char *nstr;
>  char *endptr;
>
> And I will, if you think it's better.

Thanks, yes.

> > > +    for (ptr = strtok_r(cpu, ",", &saveptr); ptr;
> > > +         ptr = strtok_r(NULL, ",", &saveptr)) {
> > 
> > A minor style complaint: If you are going to split two of these three
> > items onto their own line, please give them all their own line.
> > 
> You mean ptr should have its own line? Like this?
> 
> +    for (ptr = strtok_r(cpu, ",", &saveptr);
> +         ptr;
> +         ptr = strtok_r(NULL, ",", &saveptr)) {
> 
> If yes, I sure can do that, although the result really looks super
> unpleasant to me (but that's a matter of taste, I guess).

IMO it makes it much easier to see which of these expressions is which
of the three items in a for(;;).  That's more important than making
the lines compact.

thanks,
Ian.

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