On 02/15/2011 01:09 AM, Michal Privoznik wrote:
> Non-digit characters were accepted in --cellno resulting in
> unwanted behavior: 'virsh freecell --cellno foo' wasn't
> rejected.
> ---
> This fix addresses bug 639587
>
> tools/virsh.c | 9 ++++++++-
> 1 files changed, 8 insertions(+), 1 deletions(-)
>
> diff --git a/tools/virsh.c b/tools/virsh.c
> index c2d165d..765566d 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -2283,7 +2283,7 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
> {
> int func_ret = FALSE;
> int ret;
> - int cell, cell_given;
> + int cell, cell_given, cellStr_given;
> unsigned long long memory;
> unsigned long long *nodes = NULL;
> int all_given;
> @@ -2295,6 +2295,13 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
>
> cell = vshCommandOptInt(cmd, "cellno", &cell_given);
> all_given = vshCommandOptBool(cmd, "all");
> + vshCommandOptString(cmd, "cellno", &cellStr_given);
Hmm. Why are we parsing "cellno" twice, once with OptInt, and again
with OptString?
We REALLY need to overhaul virsh.c to make the parsers return tri-state
information (no option present, option present and parse valid, option
present but parse invalid), and update all callers accordingly.
In fact, there's already a thread on this exact same topic:
https://www.redhat.com/archives/libvir-list/2011-January/msg00132.html
with my thoughts on a better rewrite:
https://www.redhat.com/archives/libvir-list/2011-January/msg00145.html
> +
> + if (cellStr_given && !cell_given) {
> + vshError(ctl, "%s", _("cell number must not contain any "
> + "non-digit characters."));
> + goto cleanup;
> + }
However, your patch does have the advantage over Osier's first attempt,
in that it does not affect all other callers and does not give a
misleading message for --cellno=-1.
Unless anyone else has any strong opinions on getting this fixed prior
to 0.8.8, my inclination would be to postpone this issue until
post-release (after all, it only affects users that pass invalid option
arguments; it doesn't affect valid uses of the freecell command), and go
with the full-blown rewrite of the option argument parsing routines to
better handle tri-state detection and default values.
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list