Re: [libvirt] [PATCH] bhyve: cleanup bhyveBuildNetArgStr error handling

On Mon, Nov 21, 2016 at 06:45:05PM +0300, Roman Bogorodskiy wrote: >Use 'goto cleanup'-style error handling instead of explicitly >freeing variables in every error path. >--- > src/bhyve/bhyve_command.c | 39 +++++++++++++++++---------------------- > 1 file changed, 17 insertions(+), 22 deletions(-) > >diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c >index 022b03b..4914d98 100644 >--- a/src/bhyve/bhyve_command.c >+++ b/src/bhyve/bhyve_command.c >@@ -114,9 +104,14 @@ bhyveBuildNetArgStr(const virDomainDef *def, > virCommandAddArgFormat(cmd, "%d:0,virtio-net,%s,mac=%s", > net->info.addr.pci.slot, > realifname, virMacAddrFormat(&net->mac, macaddr)); >+ >+ ret = 0; >+ cleanup: >+ VIR_FREE(brname); Okay, brname was leaked before.

>+ VIR_FREE(net->ifname); But freeing stuff from the virDomainNetDef structure on success seems wrong. Previously, all the error codepaths except if (VIR_STRDUP(realifname, "tap0") < 0) on a dry run left net->ifname at NULL, but after this patch it seems the network will not be cleaned up later even though it was created successfully. ACK if you wrap it in: if (ret < 0)