External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2025-03-07 17:28:15 +00:00
Code

virsh: detect programming errors with option parsing

Browse Source 
Noticed while reviewing another patch that had an accidental
mismatch due to refactoring.  An audit of the code showed that
very few callers of vshCommandOpt were expecting a return of
-2, indicating programmer error, and of those that DID check,
they just propagated that status to yet another caller that
did not check.  Fix this by making the code blatantly warn
the programmer, rather than silently ignoring it and possibly
doing the wrong thing downstream.

I know that we frown on assert()/abort() inside libvirtd
(libraries should NEVER kill the program that linked them),
but as virsh is an app rather than the library, and as this
is not the first use of assert() in virsh, I think this
approach is okay.

* tools/virsh.h (vshCommandOpt): Drop declaration.
* tools/virsh.c (vshCommandOpt): Make static, and add a
parameter.  Abort on programmer errors rather than making callers
repeat that logic.
(vshCommandOptInt, vshCommandOptUInt, vshCommandOptUL)
(vshCommandOptString, vshCommandOptStringReq)
(vshCommandOptLongLong, vshCommandOptULongLong)
(vshCommandOptBool): Adjust callers.

Signed-off-by: Eric Blake <eblake@redhat.com>
This commit is contained in:
Eric Blake 2013-08-16 16:07:31 -06:00
parent 74c5156f9d
commit 8aecd35126
2 changed files with 34 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

96
tools/virsh.c
View File

@ -25,6 +25,7 @@
#include <config.h> #include <config.h>
#include "virsh.h" #include "virsh.h"
#include <assert.h>
#include <stdio.h> #include <stdio.h>
#include <stdlib.h> #include <stdlib.h>
#include <string.h> #include <string.h>
@ -1341,39 +1342,45 @@ vshCommandFree(vshCmd *cmd)
* @cmd: parsed command line to search * @cmd: parsed command line to search
* @name: option name to search for * @name: option name to search for
* @opt: result of the search * @opt: result of the search
* @needData: true if option must be non-boolean
* *
* Look up an option passed to CMD by NAME. Returns 1 with *OPT set * Look up an option passed to CMD by NAME. Returns 1 with *OPT set
* to the option if found, 0 with *OPT set to NULL if the name is * to the option if found, 0 with *OPT set to NULL if the name is
* valid and the option is not required, -1 with *OPT set to NULL if * valid and the option is not required, -1 with *OPT set to NULL if
* the option is required but not present, and -2 if NAME is not valid * the option is required but not present, and assert if NAME is not
* (-2 indicates a programming error). No error messages are issued. * valid (which indicates a programming error). No error messages are
* issued if a value is returned.
*/ */
int static int
vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt) vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
bool needData)
{ {
vshCmdOpt *candidate = cmd->opts; vshCmdOpt *candidate = cmd->opts;
const vshCmdOptDef *valid = cmd->def->opts; const vshCmdOptDef *valid = cmd->def->opts;
int ret = 0;
/* See if option is valid and/or required. */
*opt = NULL;
while (valid) {
assert(valid->name);
if (STREQ(name, valid->name))
break;
valid++;
}
assert(!needData || valid->type != VSH_OT_BOOL);
if (valid->flags & VSH_OFLAG_REQ)
ret = -1;
/* See if option is present on command line. */ /* See if option is present on command line. */
while (candidate) { while (candidate) {
if (STREQ(candidate->def->name, name)) { if (STREQ(candidate->def->name, name)) {
*opt = candidate; *opt = candidate;
return 1; ret = 1;
break;
} }
candidate = candidate->next; candidate = candidate->next;
} }
return ret;
/* Option not present, see if command requires it. */
*opt = NULL;
while (valid) {
if (!valid->name)
break;
if (STREQ(name, valid->name))
return (valid->flags & VSH_OFLAG_REQ) == 0 ? 0 : -1;
valid++;
}
/* If we got here, the name is unknown. */
return -2;
} }
/** /**
@ -1394,14 +1401,9 @@ vshCommandOptInt(const vshCmd *cmd, const char *name, int *value)
vshCmdOpt *arg; vshCmdOpt *arg;
int ret; int ret;
ret = vshCommandOpt(cmd, name, &arg); ret = vshCommandOpt(cmd, name, &arg, true);
if (ret <= 0) if (ret <= 0)
return ret; return ret;
if (!arg->data) {
/* only possible on bool, but if name is bool, this is a
* programming bug */
return -2;
}
if (virStrToLong_i(arg->data, NULL, 10, value) < 0) if (virStrToLong_i(arg->data, NULL, 10, value) < 0)
return -1; return -1;
@ -1424,14 +1426,9 @@ vshCommandOptUInt(const vshCmd *cmd, const char *name, unsigned int *value)
vshCmdOpt *arg; vshCmdOpt *arg;
int ret; int ret;
ret = vshCommandOpt(cmd, name, &arg); ret = vshCommandOpt(cmd, name, &arg, true);
if (ret <= 0) if (ret <= 0)
return ret; return ret;
if (!arg->data) {
/* only possible on bool, but if name is bool, this is a
* programming bug */
return -2;
}
if (virStrToLong_ui(arg->data, NULL, 10, value) < 0) if (virStrToLong_ui(arg->data, NULL, 10, value) < 0)
return -1; return -1;
@ -1454,14 +1451,9 @@ vshCommandOptUL(const vshCmd *cmd, const char *name, unsigned long *value)
vshCmdOpt *arg; vshCmdOpt *arg;
int ret; int ret;
ret = vshCommandOpt(cmd, name, &arg); ret = vshCommandOpt(cmd, name, &arg, true);
if (ret <= 0) if (ret <= 0)
return ret; return ret;
if (!arg->data) {
/* only possible on bool, but if name is bool, this is a
* programming bug */
return -2;
}
if (virStrToLong_ul(arg->data, NULL, 10, value) < 0) if (virStrToLong_ul(arg->data, NULL, 10, value) < 0)
return -1; return -1;
@ -1486,14 +1478,9 @@ vshCommandOptString(const vshCmd *cmd, const char *name, const char **value)
vshCmdOpt *arg; vshCmdOpt *arg;
int ret; int ret;
ret = vshCommandOpt(cmd, name, &arg); ret = vshCommandOpt(cmd, name, &arg, true);
if (ret <= 0) if (ret <= 0)
return ret; return ret;
if (!arg->data) {
/* only possible on bool, but if name is bool, this is a
* programming bug */
return -2;
}
if (!*arg->data && !(arg->def->flags & VSH_OFLAG_EMPTY_OK)) { if (!*arg->data && !(arg->def->flags & VSH_OFLAG_EMPTY_OK)) {
return -1; return -1;
@ -1528,21 +1515,14 @@ vshCommandOptStringReq(vshControl *ctl,
/* clear out the value */ /* clear out the value */
*value = NULL; *value = NULL;
ret = vshCommandOpt(cmd, name, &arg); ret = vshCommandOpt(cmd, name, &arg, true);
/* option is not required and not present */ /* option is not required and not present */
if (ret == 0) if (ret == 0)
return 0; return 0;
/* this should not be propagated here, just to be sure */ /* this should not be propagated here, just to be sure */
if (ret == -1) if (ret == -1)
error = N_("Mandatory option not present"); error = N_("Mandatory option not present");
else if (!*arg->data && !(arg->def->flags & VSH_OFLAG_EMPTY_OK))
if (ret == -2)
error = N_("Programming error: Invalid option name");
if (!arg->data)
error = N_("Programming error: Requested option is a boolean");
if (arg->data && !*arg->data && !(arg->def->flags & VSH_OFLAG_EMPTY_OK))
error = N_("Option argument is empty"); error = N_("Option argument is empty");
if (error) { if (error) {
@ -1570,14 +1550,9 @@ vshCommandOptLongLong(const vshCmd *cmd, const char *name,
vshCmdOpt *arg; vshCmdOpt *arg;
int ret; int ret;
ret = vshCommandOpt(cmd, name, &arg); ret = vshCommandOpt(cmd, name, &arg, true);
if (ret <= 0) if (ret <= 0)
return ret; return ret;
if (!arg->data) {
/* only possible on bool, but if name is bool, this is a
* programming bug */
return -2;
}
if (virStrToLong_ll(arg->data, NULL, 10, value) < 0) if (virStrToLong_ll(arg->data, NULL, 10, value) < 0)
return -1; return -1;
@ -1600,14 +1575,9 @@ vshCommandOptULongLong(const vshCmd *cmd, const char *name,
vshCmdOpt *arg; vshCmdOpt *arg;
int ret; int ret;
ret = vshCommandOpt(cmd, name, &arg); ret = vshCommandOpt(cmd, name, &arg, true);
if (ret <= 0) if (ret <= 0)
return ret; return ret;
if (!arg->data) {
/* only possible on bool, but if name is bool, this is a
* programming bug */
return -2;
}
if (virStrToLong_ull(arg->data, NULL, 10, value) < 0) if (virStrToLong_ull(arg->data, NULL, 10, value) < 0)
return -1; return -1;
@ -1660,7 +1630,7 @@ vshCommandOptBool(const vshCmd *cmd, const char *name)
{ {
vshCmdOpt *dummy; vshCmdOpt *dummy;
return vshCommandOpt(cmd, name, &dummy) == 1; return vshCommandOpt(cmd, name, &dummy, false) == 1;
} }
/** /**

5
tools/virsh.h
View File

@ -1,7 +1,7 @@
/* /*
* virsh.h: a shell to exercise the libvirt API * virsh.h: a shell to exercise the libvirt API
* *
* Copyright (C) 2005, 2007-2012 Red Hat, Inc. * Copyright (C) 2005, 2007-2013 Red Hat, Inc.
* *
* This library is free software; you can redistribute it and/or * This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public * modify it under the terms of the GNU Lesser General Public
@ -262,9 +262,6 @@ bool vshCmddefHelp(vshControl *ctl, const char *name);
const vshCmdGrp *vshCmdGrpSearch(const char *grpname); const vshCmdGrp *vshCmdGrpSearch(const char *grpname);
bool vshCmdGrpHelp(vshControl *ctl, const char *name); bool vshCmdGrpHelp(vshControl *ctl, const char *name);
int vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
ATTRIBUTE_RETURN_CHECK;
int vshCommandOptInt(const vshCmd *cmd, const char *name, int *value) int vshCommandOptInt(const vshCmd *cmd, const char *name, int *value)
ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
int vshCommandOptUInt(const vshCmd *cmd, const char *name, int vshCommandOptUInt(const vshCmd *cmd, const char *name,