[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/2] virsh: Pass vshControl to all vshCommandOpt*() calls.



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>


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]