LGTM, thanks!

-Yi-Hung

On Mon, Apr 24, 2017 at 9:44 AM, Ben Pfaff <[email protected]> wrote:
> 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