Re: [libvirt] [PATCH v3 10/11] util: netdevopenvswitch: use VIR_AUTOPTR for aggregate types

[Erik Skultety]

On Thu, Aug 09, 2018 at 09:42:18AM +0530, Sukrit Bhatnagar wrote:
> By making use of GNU C's cleanup attribute handled by the
> VIR_AUTOPTR macro for declaring aggregate pointer variables,
> majority of the calls to *Free functions can be dropped, which
> in turn leads to getting rid of most of our cleanup sections.
>
> Signed-off-by: Sukrit Bhatnagar 
> ---
Reviewed-by: Erik Skultety 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 10/11] util: netdevopenvswitch: use VIR_AUTOPTR for aggregate types

[Sukrit Bhatnagar]

By making use of GNU C's cleanup attribute handled by the
VIR_AUTOPTR macro for declaring aggregate pointer variables,
majority of the calls to *Free functions can be dropped, which
in turn leads to getting rid of most of our cleanup sections.

Signed-off-by: Sukrit Bhatnagar 
---
 src/util/virnetdevopenvswitch.c | 80 ++---
 1 file changed, 27 insertions(+), 53 deletions(-)

diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index 9a9435f..a5de541 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -144,11 +144,10 @@ int virNetDevOpenvswitchAddPort(const char *brname, const 
char *ifname,
virNetDevVPortProfilePtr ovsport,
virNetDevVlanPtr virtVlan)
 {
-int ret = -1;
-virCommandPtr cmd = NULL;
 char macaddrstr[VIR_MAC_STRING_BUFLEN];
 char ifuuidstr[VIR_UUID_STRING_BUFLEN];
 char vmuuidstr[VIR_UUID_STRING_BUFLEN];
+VIR_AUTOPTR(virCommand) cmd = NULL;
 VIR_AUTOFREE(char *) attachedmac_ex_id = NULL;
 VIR_AUTOFREE(char *) ifaceid_ex_id = NULL;
 VIR_AUTOFREE(char *) profile_ex_id = NULL;
@@ -160,17 +159,17 @@ int virNetDevOpenvswitchAddPort(const char *brname, const 
char *ifname,
 
 if (virAsprintf(_ex_id, "external-ids:attached-mac=\"%s\"",
 macaddrstr) < 0)
-goto cleanup;
+return -1;
 if (virAsprintf(_ex_id, "external-ids:iface-id=\"%s\"",
 ifuuidstr) < 0)
-goto cleanup;
+return -1;
 if (virAsprintf(_ex_id, "external-ids:vm-id=\"%s\"",
 vmuuidstr) < 0)
-goto cleanup;
+return -1;
 if (ovsport->profileID[0] != '\0') {
 if (virAsprintf(_ex_id, "external-ids:port-profile=\"%s\"",
 ovsport->profileID) < 0)
-goto cleanup;
+return -1;
 }
 
 cmd = virCommandNew(OVSVSCTL);
@@ -179,7 +178,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const 
char *ifname,
  ifname, "--", "add-port", brname, ifname, NULL);
 
 if (virNetDevOpenvswitchConstructVlans(cmd, virtVlan) < 0)
-goto cleanup;
+return -1;
 
 if (ovsport->profileID[0] == '\0') {
 virCommandAddArgList(cmd,
@@ -204,13 +203,10 @@ int virNetDevOpenvswitchAddPort(const char *brname, const 
char *ifname,
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to add port %s to OVS bridge %s"),
ifname, brname);
-goto cleanup;
+return -1;
 }
 
-ret = 0;
- cleanup:
-virCommandFree(cmd);
-return ret;
+return 0;
 }
 
 /**
@@ -223,8 +219,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const 
char *ifname,
  */
 int virNetDevOpenvswitchRemovePort(const char *brname ATTRIBUTE_UNUSED, const 
char *ifname)
 {
-int ret = -1;
-virCommandPtr cmd = NULL;
+VIR_AUTOPTR(virCommand) cmd = NULL;
 
 cmd = virCommandNew(OVSVSCTL);
 virNetDevOpenvswitchAddTimeout(cmd);
@@ -233,13 +228,10 @@ int virNetDevOpenvswitchRemovePort(const char *brname 
ATTRIBUTE_UNUSED, const ch
 if (virCommandRun(cmd, NULL) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to delete port %s from OVS"), ifname);
-goto cleanup;
+return -1;
 }
 
-ret = 0;
- cleanup:
-virCommandFree(cmd);
-return ret;
+return 0;
 }
 
 /**
@@ -253,9 +245,8 @@ int virNetDevOpenvswitchRemovePort(const char *brname 
ATTRIBUTE_UNUSED, const ch
  */
 int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname)
 {
-virCommandPtr cmd = NULL;
 size_t len;
-int ret = -1;
+VIR_AUTOPTR(virCommand) cmd = NULL;
 
 cmd = virCommandNew(OVSVSCTL);
 virNetDevOpenvswitchAddTimeout(cmd);
@@ -269,7 +260,7 @@ int virNetDevOpenvswitchGetMigrateData(char **migrate, 
const char *ifname)
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to run command to get OVS port data for "
  "interface %s"), ifname);
-goto cleanup;
+return -1;
 }
 
 /* Wipeout the newline, if it exists */
@@ -277,10 +268,7 @@ int virNetDevOpenvswitchGetMigrateData(char **migrate, 
const char *ifname)
 if (len > 0)
 (*migrate)[len - 1] = '\0';
 
-ret = 0;
- cleanup:
-virCommandFree(cmd);
-return ret;
+return 0;
 }
 
 /**
@@ -294,8 +282,7 @@ int virNetDevOpenvswitchGetMigrateData(char **migrate, 
const char *ifname)
  */
 int virNetDevOpenvswitchSetMigrateData(char *migrate, const char *ifname)
 {
-virCommandPtr cmd = NULL;
-int ret = -1;
+VIR_AUTOPTR(virCommand) cmd = NULL;
 
 if (!migrate) {
 VIR_DEBUG("No OVS port data for interface %s", ifname);
@@ -312,13 +299,10 @@ int virNetDevOpenvswitchSetMigrateData(char *migrate, 
const char *ifname)