Ján Tomko wrote:
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.
Yeah, I've noticed that while working on the e1000 support patches.
+ 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)
Pushed with this change squashed, thanks! Roman Bogorodskiy