On Mon, Mar 25, 2019 at 03:48:59PM +0000, Daniel P. Berrangé wrote:
On Mon, Mar 25, 2019 at 04:12:34PM +0100, Martin Kletzander wrote:
> On Mon, Mar 25, 2019 at 10:03:31AM +0100, Peter Krempa wrote:
> > On Mon, Mar 25, 2019 at 09:15:23 +0100, Erik Skultety wrote:
> > > On Mon, Mar 25, 2019 at 09:14:38AM +0100, Erik Skultety wrote:
> > > > You're missing:
> > > > - commit message explaining the change
> > > > - Your full name as author
> > > > - compliance with developer certificate of origin, see [1]
> >
> >
https://www.redhat.com/archives/libvir-list/2019-March/msg01148.html
> >
> > > >
> > > > On Sat, Mar 23, 2019 at 08:34:42PM +0400, Humaid wrote:
> > > > > ---
> > > > > src/qemu/qemu_tpm.c | 6 +++---
> > > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
> > > > > index 835a9caf46..b60e443f14 100644
> > > > > --- a/src/qemu/qemu_tpm.c
> > > > > +++ b/src/qemu/qemu_tpm.c
> > > > > @@ -834,16 +834,16 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr
driver,
> > > > >
> > > > >
> > > > > int
> > > > > -qemuExtTPMStart(virQEMUDriverPtr driver,
> > > > > - virDomainObjPtr vm,
> > > > > +qemuExtTPMStart(virDomainObjPtr vm,
> > > > > qemuDomainLogContextPtr logCtxt)
> > > > > {
> > > > > int ret = 0;
> > > > > + qemuDomainObjPrivatePtr priv = vm->privateData;
> > > >
> > > > A quick grep through the code base shows that we could do this at many
more
> > > > places actually.
> >
> > Daniel pointed out that it's not actually worth doing as a separate
> > cleanup:
> >
> >
https://www.redhat.com/archives/libvir-list/2019-March/msg01147.html
>
> For cleaning things up I think this makes sense, and I understood the above as
> Daniel not being convinced because there was no reasoning behind that at all (no
> commit message, etc.), hopefully I am not mistaken.
No, I was saying I didn't see any point in doing this change. I don't
think it is a benefit to reduce the parameter count in exchange for
increasing the local variable count. This just feels like repainting
the bikeshed a different colour.
OK, sorry for the misunderstanding then. Although it starts to make sense when
the parameters bubble through more than one function, the function calls more functions
like this and/or the function does not need the driver pointer at all. The code would be
more concise.
Feel free to disagree and let me know if I should remove this from the bite
sized tasks on the wiki. Pity we didn't reach that conclusion earlier then.