2010/12/21 PaweÅ KrzeÅniak <pawel.krzesniak@xxxxxxxxx>:
> 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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list