On Thu, 2015-05-21 at 13:22 +0200, Peter Krempa wrote:
> -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.
It did, when I first wrote the patch, when the value for a required
option was not found; then I realized that error condition was already
taken care of in vshCommandCheckOpts() and I removed the check.
I agree that the vshControl argument can be removed. I'll get rid of it.
> -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'd rather keep it so that after the change all vshCommandOpt*() calls
are consistent. I don't see the harm in doing so, but maybe I'm missing
something?
Cheers.
--
Andrea Bolognani <abologna(a)redhat.com>