Re: [ovs-dev] [PATCH 2/2] Fix memory leak in nbctl_lrp_add.

[Ben Pfaff]

On Mon, Oct 03, 2016 at 08:19:09AM -0700, nickcooper-zhangtonghao wrote:
> Signed-off-by: nickcooper-zhangtonghao 
> <nickcooper-zhangtong...@opencloud.tech>
> ---
>  ovn/utilities/ovn-nbctl.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> index 563c6ec..71cffda 100644
> --- a/ovn/utilities/ovn-nbctl.c
> +++ b/ovn/utilities/ovn-nbctl.c
> @@ -1810,6 +1810,8 @@ nbctl_lrp_add(struct ctl_context *ctx)
>          sset_add_array(&orig_networks, lrp->networks, lrp->n_networks);
>  
>          if (!sset_equals(&orig_networks, &new_networks)) {
> +            sset_destroy(&orig_networks);
> +            sset_destroy(&new_networks);
>              ctl_fatal("%s: port already exists with different network",
>                        lrp_name);
>          }

Thanks for the patch.

I didn't like how this duplicated the sset_destroy() calls in two places
so close together, so I changed it to:

diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 2148665..b23cccf 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -1809,14 +1809,14 @@ nbctl_lrp_add(struct ctl_context *ctx)
         struct sset orig_networks = SSET_INITIALIZER(&orig_networks);
         sset_add_array(&orig_networks, lrp->networks, lrp->n_networks);
 
-        if (!sset_equals(&orig_networks, &new_networks)) {
+        bool same_networks = sset_equals(&orig_networks, &new_networks);
+        sset_destroy(&orig_networks);
+        sset_destroy(&new_networks);
+        if (!same_networks) {
             ctl_fatal("%s: port already exists with different network",
                       lrp_name);
         }
 
-        sset_destroy(&orig_networks);
-        sset_destroy(&new_networks);
-
         /* Special-case sanity-check of peer ports. */
         const char *peer = NULL;
         for (int i = 0; i < n_settings; i++) {

and applied to master.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev