[ovs-dev] ovn-controller: Fix leak in patched_datapaths processing.

[Flavio Fernandes]

> On Sep 1, 2016, at 12:42 PM, Ben Pfaff <blp at ovn.org> wrote:
> 
> Nothing freed 'key', which was dynamically allocated.  This commit changes
> 'key' so that it is no longer dynamically allocated.
> 
> Reported-by: Ryan Moats <rmoats at us.ibm.com>
> Signed-off-by: Ben Pfaff <blp at ovn.org>


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


> ---
> ovn/controller/ovn-controller.c | 4 ++--
> ovn/controller/ovn-controller.h | 2 +-
> ovn/controller/patch.c          | 4 +---
> ovn/controller/physical.c       | 4 +---
> ovn/lib/ovn-util.c              | 4 ++--
> ovn/lib/ovn-util.h              | 3 ++-
> 6 files changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
> index c2e667b..5f2f90a 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -249,8 +249,8 @@ update_ct_zones(struct sset *lports, struct hmap *patched_datapaths,
>             continue;
>         }
> 
> -        char *dnat = alloc_nat_zone_key(pd->key, "dnat");
> -        char *snat = alloc_nat_zone_key(pd->key, "snat");
> +        char *dnat = alloc_nat_zone_key(&pd->key, "dnat");
> +        char *snat = alloc_nat_zone_key(&pd->key, "snat");
>         sset_add(&all_users, dnat);
>         sset_add(&all_users, snat);
>         free(dnat);
> diff --git a/ovn/controller/ovn-controller.h b/ovn/controller/ovn-controller.h
> index 470b1f5..c1e06ca 100644
> --- a/ovn/controller/ovn-controller.h
> +++ b/ovn/controller/ovn-controller.h
> @@ -51,7 +51,7 @@ struct local_datapath *get_local_datapath(const struct hmap *,
>  * with at least one logical patch port binding. */
> struct patched_datapath {
>     struct hmap_node hmap_node;
> -    char *key;  /* Holds the uuid of the corresponding datapath. */
> +    struct uuid key;   /* UUID of the corresponding datapath. */
>     bool local; /* 'True' if the datapath is for gateway router. */
>     bool stale; /* 'True' if the datapath is not referenced by any patch
>                  * port. */
> diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c
> index c8e47b4..c9a5dd9 100644
> --- a/ovn/controller/patch.c
> +++ b/ovn/controller/patch.c
> @@ -265,8 +265,7 @@ add_patched_datapath(struct hmap *patched_datapaths,
> 
>     pd = xzalloc(sizeof *pd);
>     pd->local = local;
> -    pd->key = xasprintf(UUID_FMT,
> -                        UUID_ARGS(&binding_rec->datapath->header_.uuid));
> +    pd->key = binding_rec->datapath->header_.uuid;
>     /* stale is set to false. */
>     hmap_insert(patched_datapaths, &pd->hmap_node,
>                 binding_rec->datapath->tunnel_key);
> @@ -294,7 +293,6 @@ add_logical_patch_ports_postprocess(struct hmap *patched_datapaths)
>                         patched_datapaths) {
>         if (pd_cur_node->stale == true) {
>             hmap_remove(patched_datapaths, &pd_cur_node->hmap_node);
> -            free(pd_cur_node->key);
>             free(pd_cur_node);
>         }
>     }
> diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
> index 57276fa..df6990a 100644
> --- a/ovn/controller/physical.c
> +++ b/ovn/controller/physical.c
> @@ -300,11 +300,9 @@ consider_port_binding(enum mf_field_id mff_ovn_geneve,
>         }
> 
>         int zone_id_dnat, zone_id_snat;
> -        char *key = xasprintf(UUID_FMT,
> -                              UUID_ARGS(&binding->datapath->header_.uuid));
> +        const struct uuid *key = &binding->datapath->header_.uuid;
>         char *dnat = alloc_nat_zone_key(key, "dnat");
>         char *snat = alloc_nat_zone_key(key, "snat");
> -        free(key);
> 
>         zone_id_dnat = simap_get(ct_zones, dnat);
>         if (zone_id_dnat) {
> diff --git a/ovn/lib/ovn-util.c b/ovn/lib/ovn-util.c
> index 94feadb..5dbc138 100644
> --- a/ovn/lib/ovn-util.c
> +++ b/ovn/lib/ovn-util.c
> @@ -198,9 +198,9 @@ destroy_lport_addresses(struct lport_addresses *laddrs)
>  *
>  * It is the caller's responsibility to free the allocated memory. */
> char *
> -alloc_nat_zone_key(const char *key, const char *type)
> +alloc_nat_zone_key(const struct uuid *key, const char *type)
> {
> -    return xasprintf("%s_%s", key, type);
> +    return xasprintf(UUID_FMT"_%s", UUID_ARGS(key), type);
> }
> 
> const char *
> diff --git a/ovn/lib/ovn-util.h b/ovn/lib/ovn-util.h
> index 22bb06d..2329111 100644
> --- a/ovn/lib/ovn-util.h
> +++ b/ovn/lib/ovn-util.h
> @@ -19,6 +19,7 @@
> #include "lib/packets.h"
> 
> struct nbrec_logical_router_port;
> +struct uuid;
> 
> struct ipv4_netaddr {
>     ovs_be32 addr;            /* 192.168.10.123 */
> @@ -58,7 +59,7 @@ bool extract_lrp_networks(const struct nbrec_logical_router_port *,
>                           struct lport_addresses *);
> void destroy_lport_addresses(struct lport_addresses *);
> 
> -char *alloc_nat_zone_key(const char *key, const char *type);
> +char *alloc_nat_zone_key(const struct uuid *key, const char *type);
> 
> const char *default_nb_db(void);
> const char *default_sb_db(void);