[ovs-dev] [PATCH] ovn: fix ovn-northd leaks in build_acl

[Ryan Moats]

"dev" <dev-bounces at openvswitch.org> wrote on 08/19/2016 08:06:50 PM:

> From: Ramu Ramamurthy <ramu.ramamurthy at gmail.com>
> To: dev at openvswitch.org
> Cc: Suryanarayan Ramamurthy/San Jose/IBM at IBMUS
> Date: 08/19/2016 08:07 PM
> Subject: [ovs-dev] [PATCH] ovn: fix ovn-northd leaks in build_acl
> Sent by: "dev" <dev-bounces at openvswitch.org>
>
> From: Ramu Ramamurthy <ramu.ramamurthy at us.ibm.com>
>
> The following leaks are due to missing ds_destroy in a few
> places in build_acl.
>
> 5,850 bytes in 50 blocks are definitely lost in loss record 93 of 93
>    at 0x4C29BFD: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-
> amd64-linux.so)
>    by 0x4C2BACB: realloc (in /usr/lib64/valgrind/vgpreload_memcheck-
> amd64-linux.so)
>    by 0x449507: xrealloc (util.c:123)
>    by 0x42CC73: ds_reserve (dynamic-string.c:63)
>    by 0x42D08F: ds_put_format_valist (dynamic-string.c:161)
>    by 0x42D176: ds_put_format (dynamic-string.c:142)
>    by 0x40D380: build_acls (ovn-northd.c:2320)
>    by 0x40D380: build_lswitch_flows.constprop.36 (ovn-northd.c:2472)
>    by 0x4072D9: build_lflows (ovn-northd.c:3845)
>    by 0x4072D9: ovnnb_db_run (ovn-northd.c:3971)
>    by 0x4072D9: main (ovn-northd.c:4375)
>
> 9,360 bytes in 72 blocks are definitely lost in loss record 93 of 93
>    at 0x4C29BFD: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-
> amd64-linux.so)
>    by 0x4C2BACB: realloc (in /usr/lib64/valgrind/vgpreload_memcheck-
> amd64-linux.so)
>    by 0x449507: xrealloc (util.c:123)
>    by 0x42CC73: ds_reserve (dynamic-string.c:63)
>    by 0x42D08F: ds_put_format_valist (dynamic-string.c:161)
>    by 0x42D176: ds_put_format (dynamic-string.c:142)
>    by 0x40D505: build_acls (ovn-northd.c:2346)
>    by 0x40D505: build_lswitch_flows.constprop.36 (ovn-northd.c:2472)
>    by 0x4072D9: build_lflows (ovn-northd.c:3845)
>    by 0x4072D9: ovnnb_db_run (ovn-northd.c:3971)
>    by 0x4072D9: main (ovn-northd.c:4375)
> ---
>  ovn/northd/ovn-northd.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 625937d..756d188 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -2324,6 +2324,7 @@ build_acls(struct ovn_datapath *od, struct hmap
*lflows)
>                  ovn_lflow_add(
>                      lflows, od, S_SWITCH_OUT_ACL, 34000, ds_cstr
(&match),
>                      actions);
> +                ds_destroy(&match);
>              }
>          }
>
> @@ -2350,6 +2351,7 @@ build_acls(struct ovn_datapath *od, struct hmap
*lflows)
>                  ovn_lflow_add(
>                      lflows, od, S_SWITCH_OUT_ACL, 34000, ds_cstr
(&match),
>                      actions);
> +                ds_destroy(&match);
>              }
>          }
>      }
> --

Looks good to me on inspection, but I've not tried it out...

Acked-by: Ryan Moats <rmoats at us.ibm.com>