All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Ian Campbell <Ian.Campbell@eu.citrix.com>,
	Dario Faggioli <dario.faggioli@citrix.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>
Subject: Re: [PATCH] xl: convert vcpuid to signed in main_vcpupin()
Date: Wed, 3 Sep 2014 17:50:18 +0100	[thread overview]
Message-ID: <540746CA.4080407@citrix.com> (raw)
In-Reply-To: <1409760577.15505.13.camel@kazak.uk.xensource.com>

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 <dario.faggioli@citrix.com>
>>>> ---
>>>>  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 century should be using atoi() for input
conversion/sanitisation, given strto*() and friends in C99.

As a prefect example of why, try running `xenwatchdogd --help`

~Andrew

      parent reply	other threads:[~2014-09-03 16:50 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-20 15:36 [PATCH] xl: convert vcpuid to signed in main_vcpupin() Dario Faggioli
2014-09-03 13:52 ` Ian Campbell
2014-09-03 15:20   ` Dario Faggioli
2014-09-03 16:09     ` Ian Campbell
2014-09-03 16:42       ` Dario Faggioli
2014-09-03 16:48         ` Ian Campbell
2014-09-03 16:49       ` Andrew Cooper
2014-09-03 16:50       ` Andrew Cooper [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=540746CA.4080407@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=Ian.Campbell@eu.citrix.com \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=dario.faggioli@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.