On Thu, May 21, 2015 at 11:40:26 +0200, Andrea Bolognani wrote:
This will allow us to use vshError() to report errors from inside vshCommandOpt*(), instead of replicating the same logic and error messages all over the place.
We also have more context inside the vshCommandOpt*() functions, for example the actual value used on the command line, which means we can produce more detailed error messages. --- tools/virsh-domain-monitor.c | 90 +++---- tools/virsh-domain.c | 598 +++++++++++++++++++++---------------------- tools/virsh-host.c | 46 ++-- tools/virsh-interface.c | 14 +- tools/virsh-network.c | 34 +-- tools/virsh-nodedev.c | 6 +- tools/virsh-pool.c | 26 +- tools/virsh-secret.c | 8 +- tools/virsh-snapshot.c | 88 +++---- tools/virsh-volume.c | 34 +-- tools/virsh.c | 113 ++++---- tools/virsh.h | 77 +++--- 12 files changed, 571 insertions(+), 563 deletions(-)
...
diff --git a/tools/virsh.c b/tools/virsh.c index 4425774..9f94b75 100644 --- a/tools/virsh.c +++ b/tools/virsh.c
...
-vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt, +vshCommandOpt(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd, + const char *name, vshCmdOpt **opt, bool needData)
So this helper is not using ctl nor reporting any errors. Even in the next patch.
{ vshCmdOpt *candidate = cmd->opts;
...
@@ -1829,11 +1831,12 @@ vshCommandOptScaledInt(const vshCmd *cmd, const char *name, * option is present without actually using that data. */ bool -vshCommandOptBool(const vshCmd *cmd, const char *name) +vshCommandOptBool(vshControl *ctl, const vshCmd *cmd, + const char *name) { vshCmdOpt *dummy;
- return vshCommandOpt(cmd, name, &dummy, false) == 1; + return vshCommandOpt(ctl, cmd, name, &dummy, false) == 1;
And vshCommandOptBool is designed not to report any error. I'm not a big fan of changing half of virsh by changing the prototype and then not using the parameter at all.
}
/**
I didn't go through the rest of the changes, but reporting malformed integers right in the parser makes sense to me. Peter