From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH] xl: convert vcpuid to signed in main_vcpupin() Date: Wed, 3 Sep 2014 17:48:09 +0100 Message-ID: <1409762889.15505.27.camel@kazak.uk.xensource.com> References: <20140820153600.20604.97575.stgit@Solace.lan> <1409752371.23789.22.camel@kazak.uk.xensource.com> <1409757629.2673.77.camel@Solace.lan> <1409760577.15505.13.camel@kazak.uk.xensource.com> <1409762568.2673.130.camel@Solace.lan> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XPDjC-0000pf-Bl for xen-devel@lists.xenproject.org; Wed, 03 Sep 2014 16:48:14 +0000 In-Reply-To: <1409762568.2673.130.camel@Solace.lan> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Dario Faggioli Cc: xen-devel , Ian Jackson List-Id: xen-devel@lists.xenproject.org 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 > > > > > --- > > > > > 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.