]> git.ipfire.org Git - thirdparty/libvirt.git/commit
qemu_hotplug.c: don't skip cleanup on failures of qemuDomainAttachNetDevice
authorLaine Stump <laine@redhat.com>
Fri, 21 May 2021 17:03:17 +0000 (13:03 -0400)
committerLaine Stump <laine@redhat.com>
Tue, 25 May 2021 13:52:21 +0000 (09:52 -0400)
commit6731a6805ddbdd4e3a67fdb4440ee52226bb493b
tree3b2a8e8aa54067e80019e7c1a287a17f8e4461db
parent90c7b9a6446b1ac1570d685b86df84cc52c67327
qemu_hotplug.c: don't skip cleanup on failures of qemuDomainAttachNetDevice

We have many places where the earliest error returns from a function
skip any cleanup label at the bottom (the assumption being that it is
so early in the function that there isn't yet anything that needs to
be explicitly undone on failure). But in general it is a bad sign if
there are any direct "return" statements in a function at any time
after there has been a "goto cleanup" - that indicates someone thought
that an earlier point in the code had done something needing cleanup,
so we shouldn't be skipping it.

There were two occurences of a "return -1" after "goto cleanup" in
qemuDomainAttachDeviceNet(). The first of these has been around for a
very long time (since 2013) and my assumption is that the earlier
"goto cleanup" didn't exist at that time (so it was proper), and when
the code further up in the function was added, the this return -1 was
missed. The second was added during a mass change to check the return
from qemuInterfacePrepareSlirp() in several places (commit
99a1cfc43889c6d); in this case it was erroneous from the start.

Change both of these "return -1"s to "goto cleanup". Since we already
have code paths earlier in the function that goto cleanup, this should
not cause any new problem.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
src/qemu/qemu_hotplug.c