diff mbox

[ovs-dev] Fix leak in patched_datapaths processing.

Message ID 1472740164-16503-1-git-send-email-rmoats@us.ibm.com
State Superseded
Headers show

Commit Message

Ryan Moats Sept. 1, 2016, 2:29 p.m. UTC
When unpersisting patch_datapaths, missed that the key field
needed to be freed to avoid a memory leak.  This patch fixes
same.

Signed-off-by: Ryan Moats <rmoats@us.ibm.com>
---
 ovn/controller/ovn-controller.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Flaviof Sept. 1, 2016, 3:24 p.m. UTC | #1
> On Sep 1, 2016, at 10:29 AM, Ryan Moats <rmoats@us.ibm.com> wrote:
> 
> When unpersisting patch_datapaths, missed that the key field
> needed to be freed to avoid a memory leak.  This patch fixes
> same.
> 
> Signed-off-by: Ryan Moats <rmoats@us.ibm.com>

Acked-by: Flavio Fernandes <flavio@flaviof.com>

> ---
> ovn/controller/ovn-controller.c | 1 +
> 1 file changed, 1 insertion(+)
> 
> diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
> index c2e667b..c4f02bc 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -477,6 +477,7 @@ main(int argc, char *argv[])
>         HMAP_FOR_EACH_SAFE (pd_cur_node, pd_next_node, hmap_node,
>                 &patched_datapaths) {
>             hmap_remove(&patched_datapaths, &pd_cur_node->hmap_node);
> +            free(pd_cur_node->key);
>             free(pd_cur_node);
>         }
>         hmap_destroy(&patched_datapaths);
> --
> 2.7.4 (Apple Git-66)
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
Ben Pfaff Sept. 1, 2016, 4:43 p.m. UTC | #2
On Thu, Sep 01, 2016 at 09:29:24AM -0500, Ryan Moats wrote:
> When unpersisting patch_datapaths, missed that the key field
> needed to be freed to avoid a memory leak.  This patch fixes
> same.
> 
> Signed-off-by: Ryan Moats <rmoats@us.ibm.com>

I don't see a reason to be doing dynamic allocation here at all.
My counterproposal:
        https://patchwork.ozlabs.org/patch/664967/
Flaviof Sept. 1, 2016, 4:49 p.m. UTC | #3
> On Sep 1, 2016, at 12:43 PM, Ben Pfaff <blp@ovn.org> wrote:
> 
> On Thu, Sep 01, 2016 at 09:29:24AM -0500, Ryan Moats wrote:
>> When unpersisting patch_datapaths, missed that the key field
>> needed to be freed to avoid a memory leak.  This patch fixes
>> same.
>> 
>> Signed-off-by: Ryan Moats <rmoats@us.ibm.com>
> 
> I don't see a reason to be doing dynamic allocation here at all.
> My counterproposal:
>        https://patchwork.ozlabs.org/patch/664967/

ah, nice!

-- flaviof

> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
diff mbox

Patch

diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index c2e667b..c4f02bc 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -477,6 +477,7 @@  main(int argc, char *argv[])
         HMAP_FOR_EACH_SAFE (pd_cur_node, pd_next_node, hmap_node,
                 &patched_datapaths) {
             hmap_remove(&patched_datapaths, &pd_cur_node->hmap_node);
+            free(pd_cur_node->key);
             free(pd_cur_node);
         }
         hmap_destroy(&patched_datapaths);