Re: [PATCH 1 of 3] libxl: extend pCPUs specification for vcpu-pin.

[Dario Faggioli <raistlin@linux.it>]

[-- Attachment #1.1: Type: text/plain, Size: 2239 bytes --]

On Thu, 2012-01-12 at 17:34 +0000, Ian Jackson wrote: 
> Dario Faggioli writes ("[Xen-devel] [PATCH 1 of 3] libxl: extend pCPUs specification for vcpu-pin."):
> > Allow for "^<cpuid>" syntax while specifying the pCPUs list
> > during a vcpu-pin. This enables doing the following:
> > 
> >  xl vcpu-pin 1 1 0-4,^2
> ...
> > +    if (strcmp(cpu, "all")) {
> > +        for (toka = strtok(cpu, ","), i = 0; toka; toka = strtok(NULL, ","), ++i) {
> 
> OMG you used strtok.  strtok is not thread-safe.  
>
Well, that's not properly me, is it that?

hg annotate tools/libxl/xl_cmdimpl.c
...
21247:     if (strcmp(cpu, "all")) { 
21247:         for (toka = strtok(cpu, ","), i = 0; toka; toka = strtok(NULL, ","), ++i) {
21247:             cpuida = strtoul(toka, &endptr, 10);
21247:             if (toka == endptr) {
...

> strtok_r is but
> still modifies its input string - hence your need to strdup.  If you
> do want to do it this way IMO the strdup should be in vcpupin_parse
> which should take a const char*.  Are you sure this is the right
> approach to parsing this ?
> 
Again, I know that's probably not an alibi, but that's not my code! :-P
What I did is just move this from the body of vcpupin() to an helper
function, and try to add as _less_ thing as I can to support a slightly
improved syntax.

That being said, if you think I should rewrite the parsing from scratch,
I can think about it, just wanted to clarify my position. :-)

> <record type="broken">
> Your patch 2 has a number of overly long lines.
> </record>
> 
I know. What I usually do is try to comply with the coding style I find
in a file. Thus, if there are, from time to time, lines overcoming
col80, I tend to relax such constrain a bit (especially when I change
one of those lines). Anyway, for v2 I'll double check that, as I don't
like long lines either. :-)

Thanks,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-------------------------------------------------------------------
Dario Faggioli, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
PhD Candidate, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa (Italy)



[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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