From 289d25016b79ed7908a29af7ba35e18f1ec6ed0a Mon Sep 17 00:00:00 2001 From: Lou Berger Date: Wed, 10 Apr 2013 12:30:04 -0700 Subject: [PATCH] bgpd, lib: memory cleanups for valgrind, plus debug changes Description: We use valgrind memcheck quite a bit to spot leaks in our work with bgpd. In order to eliminate false positives, we added code in the exit path to release the remaining allocated memory. Bgpd startup log message now includes pid. Some little tweaks by Paul Jakma : * bgp_mplsvpn.c: (str2prefix_rd) do the cleanup in common code at the end and goto it. [DL: dropped several chunks from original commit which are obsolete by now on this tree.] --- bgpd/bgp_aspath.c | 14 +++++++++++--- bgpd/bgp_attr.c | 13 +++++++++++++ bgpd/bgp_mplsvpn.c | 33 ++++++++++++++++++--------------- bgpd/bgp_network.c | 4 +++- lib/routemap.c | 3 +++ 5 files changed, 48 insertions(+), 19 deletions(-) diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c index eca4441010..70d4f04935 100644 --- a/bgpd/bgp_aspath.c +++ b/bgpd/bgp_aspath.c @@ -100,6 +100,12 @@ assegment_data_new (int num) return (XMALLOC (MTYPE_AS_SEG_DATA, ASSEGMENT_DATA_SIZE (num, 1))); } +static void +assegment_data_free (as_t *asdata) +{ + XFREE (MTYPE_AS_SEG_DATA, asdata); +} + const char *aspath_segment_type_str[] = { "as-invalid", "as-set", @@ -136,7 +142,7 @@ assegment_free (struct assegment *seg) return; if (seg->as) - XFREE (MTYPE_AS_SEG_DATA, seg->as); + assegment_data_free (seg->as); memset (seg, 0xfe, sizeof(struct assegment)); XFREE (MTYPE_AS_SEG, seg); @@ -204,13 +210,14 @@ assegment_prepend_asns (struct assegment *seg, as_t asnum, int num) if (num >= AS_SEGMENT_MAX) return seg; /* we don't do huge prepends */ - newas = assegment_data_new (seg->length + num); + if ((newas = assegment_data_new (seg->length + num)) == NULL) + return seg; for (i = 0; i < num; i++) newas[i] = asnum; memcpy (newas + num, seg->as, ASSEGMENT_DATA_SIZE (seg->length, 1)); - XFREE (MTYPE_AS_SEG_DATA, seg->as); + assegment_data_free (seg->as); seg->as = newas; seg->length += num; @@ -2111,6 +2118,7 @@ aspath_init (void) void aspath_finish (void) { + hash_clean (ashash, (void (*)(void *))aspath_free); hash_free (ashash); ashash = NULL; diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 220acb3ea8..a366a09453 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -215,6 +215,7 @@ cluster_init (void) static void cluster_finish (void) { + hash_clean (cluster_hash, (void (*)(void *))cluster_free); hash_free (cluster_hash); cluster_hash = NULL; } @@ -413,6 +414,7 @@ transit_init (void) static void transit_finish (void) { + hash_clean (transit_hash, (void (*)(void *))transit_free); hash_free (transit_hash); transit_hash = NULL; } @@ -661,9 +663,20 @@ attrhash_init (void) attrhash = hash_create (attrhash_key_make, attrhash_cmp); } +/* + * special for hash_clean below + */ +static void +attr_vfree (void *attr) +{ + bgp_attr_extra_free ((struct attr *)attr); + XFREE (MTYPE_ATTR, attr); +} + static void attrhash_finish (void) { + hash_clean(attrhash, attr_vfree); hash_free (attrhash); attrhash = NULL; } diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index 86e8788351..0b816f89e9 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -300,11 +300,12 @@ bgp_nlri_parse_vpn (struct peer *peer, struct attr *attr, int str2prefix_rd (const char *str, struct prefix_rd *prd) { - int ret; + int ret; /* ret of called functions */ + int lret; /* local ret, of this func */ char *p; char *p2; - struct stream *s; - char *half; + struct stream *s = NULL; + char *half = NULL; struct in_addr addr; s = stream_new (8); @@ -312,12 +313,13 @@ str2prefix_rd (const char *str, struct prefix_rd *prd) prd->family = AF_UNSPEC; prd->prefixlen = 64; + lret = 0; p = strchr (str, ':'); if (! p) - return 0; + goto out; if (! all_digit (p + 1)) - return 0; + goto out; half = XMALLOC (MTYPE_TMP, (p - str) + 1); memcpy (half, str, (p - str)); @@ -328,10 +330,8 @@ str2prefix_rd (const char *str, struct prefix_rd *prd) if (! p2) { if (! all_digit (half)) - { - XFREE (MTYPE_TMP, half); - return 0; - } + goto out; + stream_putw (s, RD_TYPE_AS); stream_putw (s, atoi (half)); stream_putl (s, atol (p + 1)); @@ -340,18 +340,21 @@ str2prefix_rd (const char *str, struct prefix_rd *prd) { ret = inet_aton (half, &addr); if (! ret) - { - XFREE (MTYPE_TMP, half); - return 0; - } + goto out; + stream_putw (s, RD_TYPE_IP); stream_put_in_addr (s, &addr); stream_putw (s, atol (p + 1)); } memcpy (prd->val, s->data, 8); + lret = 1; - XFREE(MTYPE_TMP, half); - return 1; +out: + if (s) + stream_free (s); + if (half) + XFREE(MTYPE_TMP, half); + return lret; } int diff --git a/bgpd/bgp_network.c b/bgpd/bgp_network.c index bceeea6489..65e2aeaf4e 100644 --- a/bgpd/bgp_network.c +++ b/bgpd/bgp_network.c @@ -473,6 +473,7 @@ bgp_bind (struct peer *peer) { #ifdef SO_BINDTODEVICE int ret; + int myerrno; char *name = NULL; /* If not bound to an interface or part of a VRF, we don't care. */ @@ -504,6 +505,7 @@ bgp_bind (struct peer *peer) ret = setsockopt (peer->fd, SOL_SOCKET, SO_BINDTODEVICE, name, strlen(name)); + myerrno = errno; if (bgpd_privs.change (ZPRIVS_LOWER) ) zlog_err ("bgp_bind: could not lower privs"); @@ -511,7 +513,7 @@ bgp_bind (struct peer *peer) if (ret < 0) { if (bgp_debug_neighbor_events (peer)) - zlog_debug ("bind to interface %s failed", name); + zlog_debug ("bind to interface %s failed, errno=%d", name, myerrno); return ret; } #endif /* SO_BINDTODEVICE */ diff --git a/lib/routemap.c b/lib/routemap.c index 6f93087ae9..1c759dc6de 100644 --- a/lib/routemap.c +++ b/lib/routemap.c @@ -1141,6 +1141,9 @@ route_map_finish (void) route_match_vec = NULL; vector_free (route_set_vec); route_set_vec = NULL; + /* cleanup route_map */ + while (route_map_master.head) + route_map_delete (route_map_master.head); } /* Routines for route map dependency lists and dependency processing */