[libvirt] [PATCH 1/2] virsh: Change integer option parsing functions to return tri-state information.
4:16 a.m.
This is needed to detect situations when optional argument was
specified with non-integer value: '--int-opt foo'.
---
tools/virsh.c | 46 +++++++++++++++++++++++++++-------------------
1 files changed, 27 insertions(+), 19 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 62fca17..e5093a2 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -252,13 +252,14 @@ static const vshCmdGrp *vshCmdGrpSearch(const char *grpname);
static int vshCmdGrpHelp(vshControl *ctl, const char *name);
static vshCmdOpt *vshCommandOpt(const vshCmd *cmd, const char *name);
-static int vshCommandOptInt(const vshCmd *cmd, const char *name, int *found);
+static int vshCommandOptInt(const vshCmd *cmd, const char *name, int *found,
+ int *opt_found);
static unsigned long vshCommandOptUL(const vshCmd *cmd, const char *name,
- int *found);
+ int *found, int *opt_found);
static char *vshCommandOptString(const vshCmd *cmd, const char *name,
int *found);
static long long vshCommandOptLongLong(const vshCmd *cmd, const char *name,
- int *found);
+ int *found, int *opt_found);
static int vshCommandOptBool(const vshCmd *cmd, const char *name);
static char *vshCommandOptArgv(const vshCmd *cmd, int count);
@@ -1602,7 +1603,7 @@ cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd,
param->type == VIR_DOMAIN_SCHED_FIELD_UINT &&
vshCommandOptBool(cmd, "weight")) {
int val;
- val = vshCommandOptInt(cmd, "weight", &found);
+ val = vshCommandOptInt(cmd, "weight", &found, NULL);
if (!found) {
vshError(ctl, "%s", _("Invalid value of weight"));
return -1;
@@ -1617,7 +1618,7 @@ cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd,
param->type == VIR_DOMAIN_SCHED_FIELD_UINT &&
vshCommandOptBool(cmd, "cap")) {
int val;
- val = vshCommandOptInt(cmd, "cap", &found);
+ val = vshCommandOptInt(cmd, "cap", &found, NULL);
if (!found) {
vshError(ctl, "%s", _("Invalid value of cap"));
return -1;
@@ -2300,7 +2301,7 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
- cell = vshCommandOptInt(cmd, "cellno", &cell_given);
+ cell = vshCommandOptInt(cmd, "cellno", &cell_given, NULL);
all_given = vshCommandOptBool(cmd, "all");
if (all_given && cell_given) {
@@ -2726,7 +2727,7 @@ cmdVcpupin(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return FALSE;
- vcpu = vshCommandOptInt(cmd, "vcpu", &vcpufound);
+ vcpu = vshCommandOptInt(cmd, "vcpu", &vcpufound, NULL);
if (!vcpufound) {
vshError(ctl, "%s", _("vcpupin: Invalid or missing vCPU
number."));
virDomainFree(dom);
@@ -2862,7 +2863,7 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return FALSE;
- count = vshCommandOptInt(cmd, "count", &count);
+ count = vshCommandOptInt(cmd, "count", &count, NULL);
if (!flags) {
if (virDomainSetVcpus(dom, count) != 0) {
@@ -2927,7 +2928,7 @@ cmdSetmem(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return FALSE;
- kilobytes = vshCommandOptUL(cmd, "kilobytes", NULL);
+ kilobytes = vshCommandOptUL(cmd, "kilobytes", NULL, NULL);
if (kilobytes <= 0) {
virDomainFree(dom);
vshError(ctl, _("Invalid value of %lu for memory size"), kilobytes);
@@ -2984,7 +2985,7 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return FALSE;
- kilobytes = vshCommandOptInt(cmd, "kilobytes", &kilobytes);
+ kilobytes = vshCommandOptInt(cmd, "kilobytes", &kilobytes, NULL);
if (kilobytes <= 0) {
virDomainFree(dom);
vshError(ctl, _("Invalid value of %d for memory size"), kilobytes);
@@ -3056,22 +3057,22 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd)
return FALSE;
hard_limit =
- vshCommandOptLongLong(cmd, "hard-limit", NULL);
+ vshCommandOptLongLong(cmd, "hard-limit", NULL, NULL);
if (hard_limit)
nparams++;
soft_limit =
- vshCommandOptLongLong(cmd, "soft-limit", NULL);
+ vshCommandOptLongLong(cmd, "soft-limit", NULL, NULL);
if (soft_limit)
nparams++;
swap_hard_limit =
- vshCommandOptLongLong(cmd, "swap-hard-limit", NULL);
+ vshCommandOptLongLong(cmd, "swap-hard-limit", NULL, NULL);
if (swap_hard_limit)
nparams++;
min_guarantee =
- vshCommandOptLongLong(cmd, "min-guarantee", NULL);
+ vshCommandOptLongLong(cmd, "min-guarantee", NULL, NULL);
if (min_guarantee)
nparams++;
@@ -3670,7 +3671,7 @@ cmdMigrate (vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptBool (cmd, "live"))
live_flag = TRUE;
- timeout = vshCommandOptInt(cmd, "timeout", &found);
+ timeout = vshCommandOptInt(cmd, "timeout", &found, NULL);
if (found) {
if (! live_flag) {
vshError(ctl, "%s", _("migrate: Unexpected timeout for offline
migration"));
@@ -3807,7 +3808,7 @@ cmdMigrateSetMaxDowntime(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return FALSE;
- downtime = vshCommandOptLongLong(cmd, "downtime", &found);
+ downtime = vshCommandOptLongLong(cmd, "downtime", &found, NULL);
if (!found || downtime < 1) {
vshError(ctl, "%s", _("migrate: Invalid downtime"));
goto done;
@@ -10796,7 +10797,7 @@ vshCommandOpt(const vshCmd *cmd, const char *name)
* Returns option as INT
*/
static int
-vshCommandOptInt(const vshCmd *cmd, const char *name, int *found)
+vshCommandOptInt(const vshCmd *cmd, const char *name, int *found, int *opt_found)
{
vshCmdOpt *arg = vshCommandOpt(cmd, name);
int res = 0, num_found = FALSE;
@@ -10811,11 +10812,13 @@ vshCommandOptInt(const vshCmd *cmd, const char *name, int
*found)
}
if (found)
*found = num_found;
+ if (opt_found)
+ *opt_found = arg ? TRUE : FALSE;
return res;
}
static unsigned long
-vshCommandOptUL(const vshCmd *cmd, const char *name, int *found)
+vshCommandOptUL(const vshCmd *cmd, const char *name, int *found, int *opt_found)
{
vshCmdOpt *arg = vshCommandOpt(cmd, name);
unsigned long res = 0;
@@ -10831,6 +10834,8 @@ vshCommandOptUL(const vshCmd *cmd, const char *name, int *found)
}
if (found)
*found = num_found;
+ if (opt_found)
+ *opt_found = arg ? TRUE : FALSE;
return res;
}
@@ -10858,7 +10863,8 @@ vshCommandOptString(const vshCmd *cmd, const char *name, int
*found)
* Returns option as long long
*/
static long long
-vshCommandOptLongLong(const vshCmd *cmd, const char *name, int *found)
+vshCommandOptLongLong(const vshCmd *cmd, const char *name, int *found,
+ int *opt_found)
{
vshCmdOpt *arg = vshCommandOpt(cmd, name);
int num_found = FALSE;
@@ -10869,6 +10875,8 @@ vshCommandOptLongLong(const vshCmd *cmd, const char *name, int
*found)
num_found = !virStrToLong_ll(arg->data, &end_p, 10, &res);
if (found)
*found = num_found;
+ if (opt_found)
+ *opt_found = arg ? TRUE : FALSE;
return res;
}
--
1.7.4
4:16 a.m.
New subject: [libvirt] [PATCH 2/2] virsh: Change optional int arguments to check for non-integer values
---
tools/virsh.c | 18 ++++++++++++++----
1 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index e5093a2..c9b4c57 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2285,7 +2285,7 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
{
int func_ret = FALSE;
int ret;
- int cell, cell_given;
+ int cell, cell_given, cell_opt_given;
unsigned long long memory;
xmlNodePtr *nodes = NULL;
unsigned long nodes_cnt;
@@ -2301,7 +2301,7 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
- cell = vshCommandOptInt(cmd, "cellno", &cell_given, NULL);
+ cell = vshCommandOptInt(cmd, "cellno", &cell_given,
&cell_opt_given);
all_given = vshCommandOptBool(cmd, "all");
if (all_given && cell_given) {
@@ -2310,6 +2310,11 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
+ if (!cell_given && cell_opt_given) {
+ vshError(ctl, "%s", _("cell number has to be a number"));
+ goto cleanup;
+ }
+
if (all_given) {
cap_xml = virConnectGetCapabilities(ctl->conn);
if (!cap_xml) {
@@ -2848,7 +2853,7 @@ static int
cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom;
- int count;
+ int count, opt_count;
int ret = TRUE;
int maximum = vshCommandOptBool(cmd, "maximum");
int config = vshCommandOptBool(cmd, "config");
@@ -2863,7 +2868,12 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return FALSE;
- count = vshCommandOptInt(cmd, "count", &count, NULL);
+ count = vshCommandOptInt(cmd, "count", &count, &opt_count);
+
+ if (!count && opt_count) {
+ vshError(ctl, "%s", _("count has to be a number"));
+ goto cleanup;
+ }
if (!flags) {
if (virDomainSetVcpus(dom, count) != 0) {
--
1.7.4
12:15 p.m.
New subject: [libvirt] [PATCH 1/2] virsh: Change integer option parsing functions to return tri-state information.
On 03/01/2011 03:16 AM, Michal Privoznik wrote:
This is needed to detect situations when optional argument was
specified with non-integer value: '--int-opt foo'.
---
tools/virsh.c | 46 +++++++++++++++++++++++++++-------------------
1 files changed, 27 insertions(+), 19 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 62fca17..e5093a2 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -252,13 +252,14 @@ static const vshCmdGrp *vshCmdGrpSearch(const char *grpname);
static int vshCmdGrpHelp(vshControl *ctl, const char *name);
static vshCmdOpt *vshCommandOpt(const vshCmd *cmd, const char *name);
-static int vshCommandOptInt(const vshCmd *cmd, const char *name, int *found);
+static int vshCommandOptInt(const vshCmd *cmd, const char *name, int *found,
+ int *opt_found);
This is awkward. You now have two optional parameters, and still can't
provide a default value easily. I'd much rather see this (as pointed
out in
https://www.redhat.com/archives/libvir-list/2011-January/msg00145.html):
int
vshCommandOptUL(const vshCmd *cmd, const char *name,
unsigned long *value) ATTRIBUTE_NONNULL(3);
Return value is <0 on failure (*value untouched), 0 when option is
absent (*value untouched), and >0 on success (*value updated). Swapping
the API like that also has the benefit that a client can specify a
non-zero default:
unsigned long value = 1024;
if (vshCommandOptUL(cmd, "arg", &value) < 0) {
error; return FALSE;
}
use value
That is - the current code returns the parse value, and with your patch
would provide tri-state information via two optional pointers; but my
preference would be to swap things and provide a tri-state return code
and put the parse value in a required pointer.
Furthermore, this should be done to _all_ of the vshCommandOpt<int>
family of functions. vshCommandOptString may be worth swapping for
consistency, since it is also a tri-state (--option not provided,
--option provided but with no [or empty] string, and --option provided
with string), although I'm not as convinced on that case as I am on the
optional integer parsing issue (--option not provided, --option provided
but parse as integer failed, --option provided and integer was parsed in
range).
Fortunately, the size of your patch proves that there weren't that many
places to change. I'll see about doing the work myself in the next 15
minutes...
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
5012
days inactive
5015
days old
2 comments
2 participants
participants (2)
-
Eric Blake -
Michal Privoznik