On Thu, Mar 31, 2022 at 09:37:42AM +0100, Daniel P. Berrangé wrote:
> On Wed, Mar 30, 2022 at 10:39:09AM -0700, Andrea Bolognani wrote:
> > On Wed, Mar 30, 2022 at 06:13:58PM +0100, Daniel P. Berrangé wrote:
> > > On Wed, Mar 30, 2022 at 09:59:11AM -0700, Andrea Bolognani wrote:
> > > > + if (virQEMUCapsGuestIsNative(hostarch, guestarch)) {
> > > > + if ((ret = virFindFileInPath("/usr/libexec/qemu-kvm")) != NULL) {
> > > > + return ret;
> > > > + }
> > > > + }
> > > > +
> > > > return ret;
> > > > }
> > >
> > > That overwrites the existing 'ret' value that we want to keep
> > > when qemu-kvm isn't present. Needs
> > >
> > > if (!ret && virQEMUCapsGuestIsNative(hostarch, guestarch)) {
> > > ...
> >
> > Does it? All previous attempts immediately return ret if it's not
> > NULL, so if we get to this point we know that there's no existing
> > value to preserve.
>
> Honestly I didn't look at the earlier context, just assumed that
> 'return ret' was meaningful. If 'ret' is always NULL, then we
> should be writing 'return NULL' rather than 'return ret' to
> make it clear we've not expected any value here.
Totally agree, but it should be a separate patch. Let me just prepare
and post it right now :)
--
Andrea Bolognani / Red Hat / Virtualization
