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 <paul.jakma@hpe.com>:

* 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.]

diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c
index eca4441010df558f684056ce6b66e60b145da3f8..70d4f04935e258393f51bc1b1c4357699c687049 100644 (file)
--- 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 220acb3ea8335030a0231c472ce559683286c63d..a366a094530ff08cfc669d5d3b010edd50b2d7fc 100644 (file)
--- 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 86e87883510f4bb819257aef42229b3312667761..0b816f89e9508f84618a9526ee4d27d5784b70c6 100644 (file)
--- 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);
-
-  XFREE(MTYPE_TMP, half);
-  return 1;
+  lret = 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 bceeea6489decbf7c7f6a78bf97921c012bad92a..65e2aeaf4e91351e5a68fec5095ea73719a1a36e 100644 (file)
--- 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 6f93087ae9f1bed22987ac8026c0e9c2877c9600..1c759dc6dedbe119cec68b14b221a7df3087768c 100644 (file)
--- 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 */