2010/12/21 Paweł Krześniak <pawel.krzesn...@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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list