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 <bogorodskiy@gmail.com>
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 <pkrempa@redhat.com> I suppose that you also want to do the same change in 1/2.