[libvirt] [PATCH] virQEMUDriverPtr parameters clean up in function qemuExtTPMStart() in /src/qemu/qemu_tpm.c

[Martin Kletzander]

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.

>Regards,
>Daniel
>-- 
>|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
>|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
>|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190326/8917a954/attachment-0001.sig>