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@redhat.com>