Message ID | CALDO+SYVkkY6e-QjvFkbNKJ4hnmmcGiy58aTdDu5rnCz=44UKw@mail.gmail.com |
---|---|
State | Not Applicable |
Headers | show |
On Sat, Dec 19, 2015 at 3:07 AM, William Tu <u9012063@gmail.com> wrote: > Hi Ben, > > Thank you for the feedback. > About the wrong free in tun-metadata.c, which one do you mean? It was this one (from the original patch): > --- a/lib/tun-metadata.c > +++ b/lib/tun-metadata.c > @@ -596,6 +596,7 @@ tun_metadata_add_entry(struct tun_table *map, uint8_t > idx, uint16_t opt_class, > > err = tun_metadata_alloc_chain(map, len, cur_chain); > if (err) { > + free(cur_chain); > tun_metadata_del_entry(map, idx); > return OFPERR_NXGTMFC_TABLE_FULL; > } The first time through the loop, cur_chain is initialized to &entry->loc.c and so shouldn't be freed on error. Later instances are malloced and could have resulted in a memory leak. The patch that Ben has applied now fixes the leak.
--- a/lib/tun-metadata.c +++ b/lib/tun-metadata.c @@ -131,7 +131,7 @@ table_free(struct tun_table *map) OVS_REQUIRES(tab_mutex) HMAP_FOR_EACH (entry, node, &map->key_hmap) { tun_metadata_del_entry(map, entry - map->entries); } - + hmap_destroy(&map->key_hmap); free(map); Or this one? --- a/lib/tun-metadata.c +++ b/lib/tun-metadata.c @@ -596,6 +596,7 @@ tun_metadata_add_entry(struct tun_table *map, uint8_t idx, uint16_t opt_class, err = tun_metadata_alloc_chain(map, len, cur_chain); if (err) { + free(cur_chain); tun_metadata_del_entry(map, idx); return OFPERR_NXGTMFC_TABLE_FULL;