Re: [Libvir] [PATCH] Check a value in vshCommandOptInt()

[Daniel Veillard <veillard redhat com>]

On Thu, Aug 16, 2007 at 05:30:36PM +0900, Masayuki Sunou wrote:
> Hi
> 
> When the option which should set a number is set a character
> which is not a number, the value is changed into 0 in libvirt.
> It causes the movement not expected.
> 
> ex)
>  # virsh vcpupin dom_name A 0,1
>   => VCPU0 is pinned without the command becoming the error.
> 
> This patch fixes it by checking the value in vshCommandOptInt().
> 
> Signed-off-by: Masayuki Sunou <fj1826dm aa jp fujitsu com>

  I looked at this, yes there is a problem. But your patch needed
some rework:
    - it didn't catch '1A' for example, you need to check that 
      *end_p is the 0 of the end of the string on return of strtol
    - the logic was confusing, setting num_found to TRUE before any
      code was run made it awkward, I changed it to start with FALSE
    - there were missing { } on the first if block
    - I also think it's better to check arg->data != NULL before getting
      the main if block to avoid troubles

I ended up with 

static int
vshCommandOptInt(vshCmd * cmd, const char *name, int *found)
{
    vshCmdOpt *arg = vshCommandOpt(cmd, name);
    int res = 0, num_found = FALSE;
    char *end_p = NULL;

    if ((arg != NULL) && (arg->data != NULL)) {
        res = strtol(arg->data, &end_p, 10);
        if ((arg->data == end_p) || (*end_p!= 0))
            num_found = FALSE;
        else
            num_found = TRUE;
    }
    if (found)
        *found = num_found;
    return res;
}

  Which I commited.
Now the exemple for pincpu you provided won't break, but it doesn't
raise an error because the argument was wrong, it just return silently.
I will try to address that separately,

  Thanks,

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard redhat com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/