In summary I'd say 'Fix' or 'Fix success return from'. The function isn't untidy, it's a real bug. Just that the only caller doesn't care.
On Sat, Apr 11, 2026 at 15:35:13 +0200, Roman Bogorodskiy wrote: > The current qemuDomainGetHostnameLease() implementation > jumps to the "endjob" label when it finds hostname. > As the label is defined after "ret = 0", > qemuDomainGetHostnameLease() returns -1 in this case. > > That works because in qemuDomainGetHostname() it is used like that: > > ... > if (qemuDomainGetHostnameLease(vm, &hostname) < 0) > goto cleanup; > > ... > > cleanup: > virDomainObjEndAPI(&vm); > return hostname; > } > > So it works, but it looks confusing. To make more consistent, > use 'break' in qemuDomainGetHostnameLease() when the hostname > is found, so it returns 0 in this case. > > Signed-off-by: Roman Bogorodskiy <[email protected]> Fixes: a4a5827c9fc396f2b1848c1d393385535b106d1a > --- > src/qemu/qemu_driver.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 2d31d4aa31..b51f644241 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -16471,7 +16471,7 @@ qemuDomainGetHostnameLease(virDomainObj *vm, > VIR_FREE(leases); > > if (*hostname) > - goto endjob; > + break; > } The function also returns 0 if no hostname was found. I'd say that's semantically okay, but in such case the function ought to initialize the 'hostname' pointer to NULL before doing anything as in such case it leaves the value un-touched. > > ret = 0; With the commit message fixed and 'hostname' initialized to NULL: Reviewed-by: Peter Krempa <[email protected]> I suppose that you also want to do the same change in 1/2.
