Re: [libvirt] [PATCH] bridge_driver: cleanup improvements in dhcpStartDhcpDaemon()
2010/12/21 Paweł Krześniak <pawel.krzesniak(a)gmail.com>:
Run VIR_FREE only for non-NULL pointers.
---
src/network/bridge_driver.c | 17 +++++++++--------
1 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index b0834ae..f2857b4 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -534,34 +534,34 @@ dhcpStartDhcpDaemon(virNetworkObjPtr network)
if (!VIR_SOCKET_IS_FAMILY(&network->def->ipAddress, AF_INET)) {
networkReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("cannot start dhcp daemon without
IPv4 address for server"));
- goto cleanup;
+ goto cleanup2;
}
ret = 0;
-cleanup:
+cleanup1:
VIR_FREE(pidfile);
virCommandFree(cmd);
+cleanup2:
return ret;
}
NACK as written, because you're complicating this function
unnecessarily. It's totally fine to call VIR_FREE on a possible NULL
pointer without checking, this is the common pattern in libvirt. Your
additional label resembles the logic of
if (pointer != NULL) {
VIR_FREE(pointer);
}
This pattern in explicitly banned in libvirt by the make syntax-check
rules and should to be simplified to
VIR_FREE(pointer);
as VIR_FREE (just like the normal free function) is safe to be called
on a NULL pointer.
If you really insists in avoiding the VIR_FREE call in the error path
when the pointer is NULL, then you could just replace your goto
cleanup2 by return -1, otherwise I suggest to leave this function as
is, because there is nothing to fix here.
Matthias