On Fri, Apr 07, 2017 at 02:43:44PM -0700, Yi-Hung Wei wrote:
> In testcase "ovn -- send gratuitous arp for NAT rules on distributed router",
> valgrind reports memory leaks as following.
>     xrealloc (util.c:123)
>     add_ipv4_netaddr.isra.0 (ovn-util.c:28)
>     extract_addresses (ovn-util.c:128)
>     extract_addresses_with_port.constprop.17 (pinctrl.c:1257)
>     consider_nat_address.isra.15 (pinctrl.c:1318)
>     get_nat_addresses_and_keys (pinctrl.c:1361)
>     send_garp_run (pinctrl.c:1402)
>     pinctrl_run (pinctrl.c:796)
>     main (ovn-controller.c:619)
> 
> Signed-off-by: Yi-Hung Wei <[email protected]>
> ---
>  ovn/controller/pinctrl.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
> index 9b408ea6c253..8063e6487b2a 100644
> --- a/ovn/controller/pinctrl.c
> +++ b/ovn/controller/pinctrl.c
> @@ -1317,6 +1317,7 @@ consider_nat_address(const char *nat_address,
>      char *lport = NULL;
>      if (!extract_addresses_with_port(nat_address, laddrs, &lport)
>          || (!lport && !strcmp(pb->type, "patch"))) {
> +        destroy_lport_addresses(laddrs);
>          free(laddrs);
>          if (lport) {
>              free(lport);
> @@ -1324,6 +1325,7 @@ consider_nat_address(const char *nat_address,
>          return;
>      } else if (lport) {
>          if (!pinctrl_is_chassis_resident(lports, chassis, lport)) {
> +            destroy_lport_addresses(laddrs);
>              free(laddrs);
>              free(lport);
>              return;

Thanks for fixing the memory leak!

This function has quite a lot of redundant code.  What do you think of
this version?

--8<--------------------------cut here-------------------------->8--

From: Yi-Hung Wei <[email protected]>
Date: Fri, 7 Apr 2017 14:43:44 -0700
Subject: [PATCH] pinctrl: Fix memory leak in consider_nat_address()

In testcase "ovn -- send gratuitous arp for NAT rules on distributed router",
valgrind reports memory leaks as following.
    xrealloc (util.c:123)
    add_ipv4_netaddr.isra.0 (ovn-util.c:28)
    extract_addresses (ovn-util.c:128)
    extract_addresses_with_port.constprop.17 (pinctrl.c:1257)
    consider_nat_address.isra.15 (pinctrl.c:1318)
    get_nat_addresses_and_keys (pinctrl.c:1361)
    send_garp_run (pinctrl.c:1402)
    pinctrl_run (pinctrl.c:796)
    main (ovn-controller.c:619)

Signed-off-by: Yi-Hung Wei <[email protected]>
Signed-off-by: Ben Pfaff <[email protected]>
---
 ovn/controller/pinctrl.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 9b408ea6c253..8c5042a590ee 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -1316,20 +1316,14 @@ consider_nat_address(const char *nat_address,
     struct lport_addresses *laddrs = xmalloc(sizeof *laddrs);
     char *lport = NULL;
     if (!extract_addresses_with_port(nat_address, laddrs, &lport)
-        || (!lport && !strcmp(pb->type, "patch"))) {
+        || (!lport && !strcmp(pb->type, "patch"))
+        || (lport && !pinctrl_is_chassis_resident(lports, chassis, lport))) {
+        destroy_lport_addresses(laddrs);
         free(laddrs);
-        if (lport) {
-            free(lport);
-        }
-        return;
-    } else if (lport) {
-        if (!pinctrl_is_chassis_resident(lports, chassis, lport)) {
-            free(laddrs);
-            free(lport);
-            return;
-        }
         free(lport);
+        return;
     }
+    free(lport);
 
     int i;
     for (i = 0; i < laddrs->n_ipv4_addrs; i++) {
-- 
2.10.2

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to