bgp: memleak in ecommunity,community,lcommunity #19673
Conversation
| struct community *old_comm = bgp_attr_get_community(&attr); | ||
| if (old_comm && !old_comm->refcnt) | ||
| community_free(&old_comm); | ||
| bgp_attr_set_community(&attr, community); |
There was a problem hiding this comment.
if this is a regular problem, why not do this inside "set_community", "set_ecommunity", etc?
There was a problem hiding this comment.
Thats the right way to do. I tried to do that, but two things I faced , 1) many places they have local free, so I need to change all of them to handle double free 2) all of bgp_attr_set_* are inline functions.., if we are fine for both then I can change accordingly.
There was a problem hiding this comment.
it is far preferable to fix the problem once and right such that people coming along behind do not re-introduce the problem or use a pattern that is broken.
There was a problem hiding this comment.
Agree, that if this can be fixed completely (bad pattern), then let's do it now.
There was a problem hiding this comment.
Thanks. I will make change accordingly.
During ecommunity_replace_linkbw, we are not freeing old ecom memory. Similar thing handled in bgp_path_info_mpath_aggregate_update Signed-off-by: Soumya Roy <[email protected]>
soumyar-roy
force-pushed
the
soumya/com_memleak
branch
from
d772d93 to
70ccaed
Compare
During ecommunity_replace_linkbw, we are not freeing old ecom memory. Similar thing handled in
bgp_path_info_mpath_aggregate_update