Re: [libvirt] [PATCH v3 09/11] util: netdevip: use VIR_AUTOPTR for aggregate types
On Thu, Aug 09, 2018 at 09:42:17AM +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
> ---
> src/util/virnetdevip.c | 130
> +++--
> 1 file changed, 51 insertions(+), 79 deletions(-)
>
> diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c
> index c6d6175..4a38a54 100644
> --- a/src/util/virnetdevip.c
> +++ b/src/util/virnetdevip.c
> @@ -168,10 +168,9 @@ virNetDevIPAddrAdd(const char *ifname,
> virSocketAddr *peer,
> unsigned int prefix)
> {
> -virSocketAddr *broadcast = NULL;
> -int ret = -1;
> -struct nl_msg *nlmsg = NULL;
> unsigned int recvbuflen;
> +VIR_AUTOPTR(virNlMsg) nlmsg = NULL;
> +VIR_AUTOPTR(virSocketAddr) broadcast = NULL;
> VIR_AUTOFREE(struct nlmsghdr *) resp = NULL;
> VIR_AUTOFREE(char *) ipStr = NULL;
> VIR_AUTOFREE(char *) peerStr = NULL;
> @@ -186,13 +185,13 @@ virNetDevIPAddrAdd(const char *ifname,
> !(peer && VIR_SOCKET_ADDR_VALID(peer))) {
> /* compute a broadcast address if this is IPv4 */
> if (VIR_ALLOC(broadcast) < 0)
> -goto cleanup;
> +return -1;
>
> if (virSocketAddrBroadcastByPrefix(addr, prefix, broadcast) < 0) {
> -virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("Failed to determine broadcast address for
> '%s/%d'"),
> - ipStr, prefix);
> -goto cleanup;
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to determine broadcast address for
> '%s/%d'"),
> + ipStr, prefix);
> +return -1;
> }
> bcastStr = virSocketAddrFormat(broadcast);
> }
> @@ -206,11 +205,11 @@ virNetDevIPAddrAdd(const char *ifname,
> if (!(nlmsg = virNetDevCreateNetlinkAddressMessage(RTM_NEWADDR, ifname,
> addr, prefix,
> broadcast, peer)))
> -goto cleanup;
> +return -1;
>
> if (virNetlinkCommand(nlmsg, , ,
>0, 0, NETLINK_ROUTE, 0) < 0)
> -goto cleanup;
> +return -1;
>
>
> if (virNetlinkGetErrorCode(resp, recvbuflen) < 0) {
> @@ -220,14 +219,10 @@ virNetDevIPAddrAdd(const char *ifname,
> peerStr ? " peer " : "", peerStr ? peerStr : "",
> bcastStr ? " bcast " : "", bcastStr ? bcastStr : "",
> ifname);
> -goto cleanup;
> +return -1;
> }
>
> -ret = 0;
> - cleanup:
> -nlmsg_free(nlmsg);
> -VIR_FREE(broadcast);
> -return ret;
> +return 0;
> }
>
>
> @@ -246,30 +241,26 @@ virNetDevIPAddrDel(const char *ifname,
> virSocketAddr *addr,
> unsigned int prefix)
> {
> -int ret = -1;
> -struct nl_msg *nlmsg = NULL;
> unsigned int recvbuflen;
> +VIR_AUTOPTR(virNlMsg) nlmsg = NULL;
> VIR_AUTOFREE(struct nlmsghdr *) resp = NULL;
>
> if (!(nlmsg = virNetDevCreateNetlinkAddressMessage(RTM_DELADDR, ifname,
> addr, prefix,
> NULL, NULL)))
> -goto cleanup;
> +return -1;
>
> if (virNetlinkCommand(nlmsg, , , 0, 0,
>NETLINK_ROUTE, 0) < 0)
> -goto cleanup;
> +return -1;
>
> if (virNetlinkGetErrorCode(resp, recvbuflen) < 0) {
> virReportError(VIR_ERR_SYSTEM_ERROR,
> _("Error removing IP address from %s"), ifname);
> -goto cleanup;
> +return -1;
> }
>
> -ret = 0;
> - cleanup:
> -nlmsg_free(nlmsg);
> -return ret;
> +return 0;
> }
>
>
> @@ -292,8 +283,6 @@ virNetDevIPRouteAdd(const char *ifname,
> virSocketAddrPtr gateway,
> unsigned int metric)
> {
> -int ret = -1;
> -struct nl_msg *nlmsg = NULL;
> struct nlmsghdr *resp = NULL;
> unsigned int recvbuflen;
> unsigned int ifindex;
> @@ -304,6 +293,7 @@ virNetDevIPRouteAdd(const char *ifname,
> int errCode;
> virSocketAddr defaultAddr;
> virSocketAddrPtr actualAddr;
> +VIR_AUTOPTR(virNlMsg) nlmsg = NULL;
> VIR_AUTOFREE(char *) toStr = NULL;
> VIR_AUTOFREE(char *) viaStr = NULL;
>
> @@ -315,10 +305,10 @@ virNetDevIPRouteAdd(const char *ifname,
> int family = VIR_SOCKET_ADDR_FAMILY(gateway);
> if (family == AF_INET) {
> if (virSocketAddrParseIPv4(,
> VIR_SOCKET_ADDR_IPV4_ALL) < 0)
> -goto cleanup;
> +return
[libvirt] [PATCH v3 09/11] util: netdevip: use VIR_AUTOPTR for aggregate types
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/virnetdevip.c | 130 +++--
1 file changed, 51 insertions(+), 79 deletions(-)
diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c
index c6d6175..4a38a54 100644
--- a/src/util/virnetdevip.c
+++ b/src/util/virnetdevip.c
@@ -168,10 +168,9 @@ virNetDevIPAddrAdd(const char *ifname,
virSocketAddr *peer,
unsigned int prefix)
{
-virSocketAddr *broadcast = NULL;
-int ret = -1;
-struct nl_msg *nlmsg = NULL;
unsigned int recvbuflen;
+VIR_AUTOPTR(virNlMsg) nlmsg = NULL;
+VIR_AUTOPTR(virSocketAddr) broadcast = NULL;
VIR_AUTOFREE(struct nlmsghdr *) resp = NULL;
VIR_AUTOFREE(char *) ipStr = NULL;
VIR_AUTOFREE(char *) peerStr = NULL;
@@ -186,13 +185,13 @@ virNetDevIPAddrAdd(const char *ifname,
!(peer && VIR_SOCKET_ADDR_VALID(peer))) {
/* compute a broadcast address if this is IPv4 */
if (VIR_ALLOC(broadcast) < 0)
-goto cleanup;
+return -1;
if (virSocketAddrBroadcastByPrefix(addr, prefix, broadcast) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to determine broadcast address for '%s/%d'"),
- ipStr, prefix);
-goto cleanup;
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to determine broadcast address for
'%s/%d'"),
+ ipStr, prefix);
+return -1;
}
bcastStr = virSocketAddrFormat(broadcast);
}
@@ -206,11 +205,11 @@ virNetDevIPAddrAdd(const char *ifname,
if (!(nlmsg = virNetDevCreateNetlinkAddressMessage(RTM_NEWADDR, ifname,
addr, prefix,
broadcast, peer)))
-goto cleanup;
+return -1;
if (virNetlinkCommand(nlmsg, , ,
0, 0, NETLINK_ROUTE, 0) < 0)
-goto cleanup;
+return -1;
if (virNetlinkGetErrorCode(resp, recvbuflen) < 0) {
@@ -220,14 +219,10 @@ virNetDevIPAddrAdd(const char *ifname,
peerStr ? " peer " : "", peerStr ? peerStr : "",
bcastStr ? " bcast " : "", bcastStr ? bcastStr : "",
ifname);
-goto cleanup;
+return -1;
}
-ret = 0;
- cleanup:
-nlmsg_free(nlmsg);
-VIR_FREE(broadcast);
-return ret;
+return 0;
}
@@ -246,30 +241,26 @@ virNetDevIPAddrDel(const char *ifname,
virSocketAddr *addr,
unsigned int prefix)
{
-int ret = -1;
-struct nl_msg *nlmsg = NULL;
unsigned int recvbuflen;
+VIR_AUTOPTR(virNlMsg) nlmsg = NULL;
VIR_AUTOFREE(struct nlmsghdr *) resp = NULL;
if (!(nlmsg = virNetDevCreateNetlinkAddressMessage(RTM_DELADDR, ifname,
addr, prefix,
NULL, NULL)))
-goto cleanup;
+return -1;
if (virNetlinkCommand(nlmsg, , , 0, 0,
NETLINK_ROUTE, 0) < 0)
-goto cleanup;
+return -1;
if (virNetlinkGetErrorCode(resp, recvbuflen) < 0) {
virReportError(VIR_ERR_SYSTEM_ERROR,
_("Error removing IP address from %s"), ifname);
-goto cleanup;
+return -1;
}
-ret = 0;
- cleanup:
-nlmsg_free(nlmsg);
-return ret;
+return 0;
}
@@ -292,8 +283,6 @@ virNetDevIPRouteAdd(const char *ifname,
virSocketAddrPtr gateway,
unsigned int metric)
{
-int ret = -1;
-struct nl_msg *nlmsg = NULL;
struct nlmsghdr *resp = NULL;
unsigned int recvbuflen;
unsigned int ifindex;
@@ -304,6 +293,7 @@ virNetDevIPRouteAdd(const char *ifname,
int errCode;
virSocketAddr defaultAddr;
virSocketAddrPtr actualAddr;
+VIR_AUTOPTR(virNlMsg) nlmsg = NULL;
VIR_AUTOFREE(char *) toStr = NULL;
VIR_AUTOFREE(char *) viaStr = NULL;
@@ -315,10 +305,10 @@ virNetDevIPRouteAdd(const char *ifname,
int family = VIR_SOCKET_ADDR_FAMILY(gateway);
if (family == AF_INET) {
if (virSocketAddrParseIPv4(, VIR_SOCKET_ADDR_IPV4_ALL)
< 0)
-goto cleanup;
+return -1;
} else {
if (virSocketAddrParseIPv6(, VIR_SOCKET_ADDR_IPV6_ALL)
< 0)
-goto cleanup;
+return -1;
}
actualAddr =
@@ -330,17 +320,17 @@ virNetDevIPRouteAdd(const char *ifname,
if (virNetDevGetIPAddressBinary(actualAddr, , ) < 0 ||
