[Libvir] [PATCH] Check a value in vshCommandOptInt()
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> Thanks, Masayuki Sunou. ---------------------------------------------------------------------- Index: src/virsh.c =================================================================== RCS file: /data/cvs/libvirt/src/virsh.c,v retrieving revision 1.95 diff -u -p -r1.95 virsh.c --- src/virsh.c 14 Aug 2007 07:07:57 -0000 1.95 +++ src/virsh.c 16 Aug 2007 07:13:46 -0000 @@ -3699,12 +3699,16 @@ static int vshCommandOptInt(vshCmd * cmd, const char *name, int *found) { vshCmdOpt *arg = vshCommandOpt(cmd, name); - int res = 0; + int res = 0, num_found = TRUE; + char *end_p=NULL; if (arg) - res = atoi(arg->data); + res = strtol(arg->data, &end_p, 10); + if (arg->data == end_p) { + num_found = FALSE; + } if (found) - *found = arg ? TRUE : FALSE; + *found = (arg && num_found) ? TRUE : FALSE; return res; } ----------------------------------------------------------------------
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/
participants (2)
-
Daniel Veillard -
Masayuki Sunou