| Message ID | 1472748178-15091-1-git-send-email-blp@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
> On Sep 1, 2016, at 12:42 PM, Ben Pfaff <blp@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@us.ibm.com> > Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Flavio Fernandes <flavio@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);
On Thu, Sep 1, 2016 at 12:42 PM, Ben Pfaff <blp@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@us.ibm.com> > Signed-off-by: Ben Pfaff <blp@ovn.org> > Acked-by: Russell Bryant <russell@ovn.org>
On Thu, Sep 01, 2016 at 12:58:52PM -0400, Russell Bryant wrote: > On Thu, Sep 1, 2016 at 12:42 PM, Ben Pfaff <blp@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@us.ibm.com> > > Signed-off-by: Ben Pfaff <blp@ovn.org> > > > > Acked-by: Russell Bryant <russell@ovn.org> Thanks Russell and Flavio, I applied this to master and branch-2.6
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);
Nothing freed 'key', which was dynamically allocated. This commit changes 'key' so that it is no longer dynamically allocated. Reported-by: Ryan Moats <rmoats@us.ibm.com> Signed-off-by: Ben Pfaff <blp@ovn.org> --- 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(-)