Re: [Xen-devel] [PATCH] xl: convert vcpuid to signed in main_vcpupin()

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On mer, 2014-09-03 at 17:09 +0100, Ian Campbell wrote:
> On Wed, 2014-09-03 at 17:20 +0200, Dario Faggioli wrote:
> > 
> > > > Signed-off-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
> > > > ---
> > > >  tools/libxl/xl_cmdimpl.c |   14 +++++++-------
> > > >  1 file changed, 7 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > > > index f1c136a..a29a579 100644
> > > > --- a/tools/libxl/xl_cmdimpl.c
> > > > +++ b/tools/libxl/xl_cmdimpl.c
> > > > @@ -4601,8 +4601,9 @@ int main_vcpupin(int argc, char **argv)
> > > >      libxl_vcpuinfo *vcpuinfo;
> > > >      libxl_bitmap cpumap_hard, cpumap_soft;;
> > > >      libxl_bitmap *soft = &cpumap_soft, *hard = &cpumap_hard;
> > > > -    uint32_t vcpuid, domid;
> > > >      const char *vcpu, *hard_str, *soft_str;
> > > > +    uint32_t domid;
> > > > +    long vcpuid;
> > > 
> > > I think an int would be sufficiently large for this.
> > > 
> > The only reason why I used a long is that the function used to do the
> > actual conversion is strtol(), which returns a long.
> 
> Can't it use atoi?
> 
From atoi(3):

"The behavior is the same as strtol(nptr, NULL, 10); except that atoi()
does not detect errors."

While the current code uses some of the strtol() error handling
capabilities:

    /* Figure out with which vCPU we are dealing with */                        
                                      
    vcpuid = strtoul(vcpu, &endptr, 10);                                        
                                      
    if (vcpu == endptr) {                                                       
                                      
        if (strcmp(vcpu, "all")) {                                              
                                      
            fprintf(stderr, "Error: Invalid argument.\n");                      
                                      
            goto out;                                                           
                                      
        }                                                                       
                                      
        vcpuid = -1;                                                            
                                      
    }

I can try to reorganize the code a bit but, without being sure that the
passed string, if not the string "all", is an actual number (which,
AFAIUI, is what strtol() gives me that atoi() doesn't), I don't think we
can be as precise in error detection/reporting as we are now (I just
double checked, and i=atoi("foo") means i becomes 0).

I can probably combine atoi() and (something like) isdigit(vcpu[0]) to
get to something similar, but at that point, I think I like strtol()
better.

Let me know.

Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

Attachment: signature.asc
Description: This is a digitally signed message part

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