From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] xl: convert vcpuid to signed in main_vcpupin() Date: Wed, 3 Sep 2014 17:49:51 +0100 Message-ID: <540746AF.100@citrix.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> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XPDkq-0000xp-QD for xen-devel@lists.xenproject.org; Wed, 03 Sep 2014 16:49:56 +0000 In-Reply-To: <1409760577.15505.13.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell , Dario Faggioli Cc: xen-devel , Ian Jackson List-Id: xen-devel@lists.xenproject.org On 03/09/14 17:09, Ian Campbell wrote: > On Wed, 2014-09-03 at 17:20 +0200, Dario Faggioli wrote: >> On mer, 2014-09-03 at 14:52 +0100, Ian Campbell wrote: >>> On Wed, 2014-08-20 at 17:36 +0200, Dario Faggioli wrote: >>>> No functional changes, it just looks more correct, considering >>>> that at some point in the function we assign -1 to it (and >>>> at some other later point we check for it to be -1), >>> I think that's more than just "looks more correct", it's just wrong to >>> compare an unsigned to -1, or to assign it. >>> >>> And there may well be a functional change since gcc is no longer allowed >>> to assume that the number is +ve and therefore cannot discard some of >>> these checks any more. We are probably just lucky that gcc isn't doing >>> so already (and unlucky that it isn't generating a warning...) >>> >> Right, and thanks for all the details. :-) >> >> Should I change the changelog, if I repost? > Yes, please. > >>>> 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? No code written this decade should be using atoi() for input sanitisation, given strto*() and friends in C99. As a prefect example of why, try running `xenwatchdogd --help` ~Andrew