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

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Wed, 2014-09-03 at 18:42 +0200, Dario Faggioli wrote:
> 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.

Yes, I'd forgotten that this wasn't a straight conversion.

It's probably easier to stick with long instead of farting around range
checking the result of strtol.

Ian.


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