[bgpd] Stability fixes including bugs 397, 492

I've spent the last several weeks working on stability fixes to bgpd.
These patches fix all of the numerous crashes, assertion failures, memory
leaks and memory stomping I could find.  Valgrind was used extensively.

Added new function bgp_exit() to help catch problems.  If "debug bgp" is
configured and bgpd exits with status of 0, statistics on remaining
lib/memory.c allocations are printed to stderr.  It is my hope that other
developers will use this to stay on top of memory issues.

Example questionable exit:

  bgpd: memstats: Current memory utilization in module LIB:
  bgpd: memstats:  Link List                     :          6
  bgpd: memstats:  Link Node                     :          5
  bgpd: memstats:  Hash                          :          8
  bgpd: memstats:  Hash Bucket                   :          2
  bgpd: memstats:  Hash Index                    :          8
  bgpd: memstats:  Work queue                    :          3
  bgpd: memstats:  Work queue item               :          2
  bgpd: memstats:  Work queue name string        :          3
  bgpd: memstats: Current memory utilization in module BGP:
  bgpd: memstats:  BGP instance                  :          1
  bgpd: memstats:  BGP peer                      :          1
  bgpd: memstats:  BGP peer hostname             :          1
  bgpd: memstats:  BGP attribute                 :          1
  bgpd: memstats:  BGP extra attributes          :          1
  bgpd: memstats:  BGP aspath                    :          1
  bgpd: memstats:  BGP aspath str                :          1
  bgpd: memstats:  BGP table                     :         24
  bgpd: memstats:  BGP node                      :          1
  bgpd: memstats:  BGP route                     :          1
  bgpd: memstats:  BGP synchronise               :          8
  bgpd: memstats:  BGP Process queue             :          1
  bgpd: memstats:  BGP node clear queue          :          1
  bgpd: memstats: NOTE: If configuration exists, utilization may be expected.

Example clean exit:

  bgpd: memstats: No remaining tracked memory utilization.

This patch fixes bug #397: "Invalid free in bgp_announce_check()".

This patch fixes bug #492: "SIGBUS in bgpd/bgp_route.c:
bgp_clear_route_node()".

My apologies for not separating out these changes into individual patches.
The complexity of doing so boggled what is left of my brain.  I hope this
is all still useful to the community.

This code has been production tested, in non-route-server-client mode, on
a linux 32-bit box and a 64-bit box.

Release/reset functions, used by bgp_exit(), added to:

  bgpd/bgp_attr.c,h
  bgpd/bgp_community.c,h
  bgpd/bgp_dump.c,h
  bgpd/bgp_ecommunity.c,h
  bgpd/bgp_filter.c,h
  bgpd/bgp_nexthop.c,h
  bgpd/bgp_route.c,h
  lib/routemap.c,h

File by file analysis:

* bgpd/bgp_aspath.c: Prevent re-use of ashash after it is released.

* bgpd/bgp_attr.c: #if removed uncalled cluster_dup().

* bgpd/bgp_clist.c,h: Allow community_list_terminate() to be called from
  bgp_exit().

* bgpd/bgp_filter.c: Fix aslist->name use without allocation check, and
  also fix memory leak.

* bgpd/bgp_main.c: Created bgp_exit() exit routine.  This function frees
  allocations made as part of bgpd initialization and, to some extent,
  configuration.  If "debug bgp" is configured, memory stats are printed
  as described above.

* bgpd/bgp_nexthop.c: zclient_new() already allocates stream for
  ibuf/obuf, so bgp_scan_init() shouldn't do it too.  Also, made it so
  zlookup is global so bgp_exit() can use it.

* bgpd/bgp_packet.c: bgp_capability_msg_parse() call to bgp_clear_route()
  adjusted to use new BGP_CLEAR_ROUTE_NORMAL flag.

* bgpd/bgp_route.h: Correct reference counter "lock" to be signed.
  bgp_clear_route() now accepts a bgp_clear_route_type of either
  BGP_CLEAR_ROUTE_NORMAL or BGP_CLEAR_ROUTE_MY_RSCLIENT.

* bgpd/bgp_route.c:
  - bgp_process_rsclient(): attr was being zero'ed and then
    bgp_attr_extra_free() was being called with it, even though it was
    never filled with valid data.

  - bgp_process_rsclient(): Make sure rsclient->group is not NULL before
    use.

  - bgp_processq_del(): Add call to bgp_table_unlock().

  - bgp_process(): Add call to bgp_table_lock().

  - bgp_update_rsclient(): memset clearing of new_attr not needed since
    declarationw with "= { 0 }" does it.  memset was already commented
    out.

  - bgp_update_rsclient(): Fix screwed up misleading indentation.

  - bgp_withdraw_rsclient(): Fix screwed up misleading indentation.

  - bgp_clear_route_node(): Support BGP_CLEAR_ROUTE_MY_RSCLIENT.

  - bgp_clear_node_queue_del(): Add call to bgp_table_unlock() and also
    free struct bgp_clear_node_queue used for work item.

  - bgp_clear_node_complete(): Do peer_unlock() after BGP_EVENT_ADD() in
    case peer is released by peer_unlock() call.

  - bgp_clear_route_table(): Support BGP_CLEAR_ROUTE_MY_RSCLIENT.  Use
    struct bgp_clear_node_queue to supply data to worker.  Add call to
    bgp_table_lock().

  - bgp_clear_route(): Add support for BGP_CLEAR_ROUTE_NORMAL or
    BGP_CLEAR_ROUTE_MY_RSCLIENT.

  - bgp_clear_route_all(): Use BGP_CLEAR_ROUTE_NORMAL.

  Bug 397 fixes:

    - bgp_default_originate()
    - bgp_announce_table()

* bgpd/bgp_table.h:
  - struct bgp_table: Added reference count.  Changed type of owner to be
    "struct peer *" rather than "void *".

  - struct bgp_node: Correct reference counter "lock" to be signed.

* bgpd/bgp_table.c:
  - Added bgp_table reference counting.

  - bgp_table_free(): Fixed cleanup code.  Call peer_unlock() on owner if
    set.

  - bgp_unlock_node(): Added assertion.

  - bgp_node_get(): Added call to bgp_lock_node() to code path that it was
    missing from.

* bgpd/bgp_vty.c:
  - peer_rsclient_set_vty(): Call peer_lock() as part of peer assignment
    to owner.  Handle failure gracefully.

  - peer_rsclient_unset_vty(): Add call to bgp_clear_route() with
    BGP_CLEAR_ROUTE_MY_RSCLIENT purpose.

* bgpd/bgp_zebra.c: Made it so zclient is global so bgp_exit() can use it.

* bgpd/bgpd.c:
  - peer_lock(): Allow to be called when status is "Deleted".

  - peer_deactivate(): Supply BGP_CLEAR_ROUTE_NORMAL purpose to
    bgp_clear_route() call.

  - peer_delete(): Common variable listnode pn.  Fix bug in which rsclient
    was only dealt with if not part of a peer group.  Call
    bgp_clear_route() for rsclient, if appropriate, and do so with
    BGP_CLEAR_ROUTE_MY_RSCLIENT purpose.

  - peer_group_get(): Use XSTRDUP() instead of strdup() for conf->host.

  - peer_group_bind(): Call bgp_clear_route() for rsclient, and do so with
    BGP_CLEAR_ROUTE_MY_RSCLIENT purpose.

  - bgp_create(): Use XSTRDUP() instead of strdup() for peer_self->host.

  - bgp_delete(): Delete peers before groups, rather than after.  And then
    rather than deleting rsclients, verify that there are none at this
    point.

  - bgp_unlock(): Add assertion.

  - bgp_free(): Call bgp_table_finish() rather than doing XFREE() itself.

* lib/command.c,h: Compiler warning fixes.  Add cmd_terminate().  Fixed
  massive leak in install_element() in which cmd_make_descvec() was being
  called more than once for the same cmd->strvec/string/doc.

* lib/log.c: Make closezlog() check fp before calling fclose().

* lib/memory.c: Catch when alloc count goes negative by using signed
  counts.  Correct #endif comment.  Add log_memstats_stderr().

* lib/memory.h: Add log_memstats_stderr().

* lib/thread.c: thread->funcname was being accessed in thread_call() after
  it had been freed.  Rearranged things so that thread_call() frees
  funcname.  Also made it so thread_master_free() cleans up cpu_record.

* lib/vty.c,h: Use global command_cr.  Add vty_terminate().

* lib/zclient.c,h: Re-enable zclient_free().

bgpd/bgp_aspath.c

@@ -1799,6 +1799,7 @@ void
 aspath_finish (void)
 {
   hash_free (ashash);
+  ashash = NULL;
   
   if (snmp_stream)
     stream_free (snmp_stream);

bgpd/bgp_attr.c

@@ -148,6 +148,7 @@ cluster_free (struct cluster_list *cluster)
   XFREE (MTYPE_CLUSTER, cluster);
 }
 
+#if 0
 static struct cluster_list *
 cluster_dup (struct cluster_list *cluster)
 {
@@ -166,6 +167,7 @@ cluster_dup (struct cluster_list *cluster)
   
   return new;
 }
+#endif
 
 static struct cluster_list *
 cluster_intern (struct cluster_list *cluster)
@@ -198,6 +200,13 @@ cluster_init (void)
 {
   cluster_hash = hash_create (cluster_hash_key_make, cluster_hash_cmp);
 }
+
+static void
+cluster_finish (void)
+{
+  hash_free (cluster_hash);
+  cluster_hash = NULL;
+}
 
 /* Unknown transit attribute. */
 static struct hash *transit_hash;
@@ -278,6 +287,13 @@ transit_init (void)
 {
   transit_hash = hash_create (transit_hash_key_make, transit_hash_cmp);
 }
+
+static void
+transit_finish (void)
+{
+  hash_free (transit_hash);
+  transit_hash = NULL;
+}
 
 /* Attribute hash routines. */
 static struct hash *attrhash;
@@ -436,6 +452,13 @@ attrhash_init (void)
 }
 
 static void
+attrhash_finish (void)
+{
+  hash_free (attrhash);
+  attrhash = NULL;
+}
+
+static void
 attr_show_all_iterator (struct hash_backet *backet, struct vty *vty)
 {
   struct attr *attr = backet->data;
@@ -2302,6 +2325,17 @@ bgp_attr_init (void)
   transit_init ();
 }
 
+void
+bgp_attr_finish (void)
+{
+  aspath_finish ();
+  attrhash_finish ();
+  community_finish ();
+  ecommunity_finish ();
+  cluster_finish ();
+  transit_finish ();
+}
+
 /* Make attribute packet. */
 void
 bgp_dump_routes_attr (struct stream *s, struct attr *attr, 

bgpd/bgp_attr.h

@@ -140,6 +140,7 @@ struct transit
 
 /* Prototypes. */
 extern void bgp_attr_init (void);
+extern void bgp_attr_finish (void);
 extern int bgp_attr_parse (struct peer *, struct attr *, bgp_size_t,
 		    struct bgp_nlri *, struct bgp_nlri *);
 extern int bgp_attr_check (struct peer *, struct attr *);

bgpd/bgp_clist.c

@@ -829,7 +829,7 @@ community_list_init (void)
 }
 
 /* Terminate community-list.  */
-static void
+void
 community_list_terminate (struct community_list_handler *ch)
 {
   struct community_list_master *cm;

bgpd/bgp_clist.h

@@ -125,6 +125,7 @@ extern struct community_list_handler *bgp_clist;
 
 /* Prototypes.  */
 extern struct community_list_handler *community_list_init (void);
+extern void community_list_terminate (struct community_list_handler *);
 
 extern int community_list_set (struct community_list_handler *ch,
 			       const char *name, const char *str, int direct,

bgpd/bgp_community.c

@@ -636,3 +636,10 @@ community_init (void)
   comhash = hash_create ((unsigned int (*) (void *))community_hash_make,
 			 (int (*) (const void *, const void *))community_cmp);
 }
+
+void
+community_finish (void)
+{
+  hash_free (comhash);
+  comhash = NULL;
+}

bgpd/bgp_community.h

@@ -52,6 +52,7 @@ struct community
 
 /* Prototypes of communities attribute functions.  */
 extern void community_init (void);
+extern void community_finish (void);
 extern void community_free (struct community *);
 extern struct community *community_uniq_sort (struct community *);
 extern struct community *community_parse (u_int32_t *, u_short);

bgpd/bgp_dump.c

@@ -865,3 +865,10 @@ bgp_dump_init (void)
   install_element (CONFIG_NODE, &dump_bgp_routes_interval_cmd);
   install_element (CONFIG_NODE, &no_dump_bgp_routes_cmd);
 }
+
+void
+bgp_dump_finish (void)
+{
+  stream_free (bgp_dump_obuf);
+  bgp_dump_obuf = NULL;
+}

bgpd/bgp_dump.h

@@ -48,6 +48,7 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
 #define TABLE_DUMP_V2_PEER_INDEX_TABLE_AS4 2
 
 extern void bgp_dump_init (void);
+extern void bgp_dump_finish (void);
 extern void bgp_dump_state (struct peer *, int, int);
 extern void bgp_dump_packet (struct peer *, int, struct stream *);
 

bgpd/bgp_ecommunity.c

@@ -262,6 +262,13 @@ ecommunity_init (void)
 {
   ecomhash = hash_create (ecommunity_hash_make, ecommunity_cmp);
 }
+
+void
+ecommunity_finish (void)
+{
+  hash_free (ecomhash);
+  ecomhash = NULL;
+}
 
 /* Extended Communities token enum. */
 enum ecommunity_token

bgpd/bgp_ecommunity.h

@@ -66,6 +66,7 @@ struct ecommunity_val
 #define ecom_length(X)    ((X)->size * ECOMMUNITY_SIZE)
 
 extern void ecommunity_init (void);
+extern void ecommunity_finish (void);
 extern void ecommunity_free (struct ecommunity *);
 extern struct ecommunity *ecommunity_parse (u_int8_t *, u_short);
 extern struct ecommunity *ecommunity_dup (struct ecommunity *);

bgpd/bgp_filter.c

@@ -181,6 +181,11 @@ as_list_new (void)
 static void
 as_list_free (struct as_list *aslist)
 {
+  if (aslist->name)
+    {
+      free (aslist->name);
+      aslist->name = NULL;
+    }
   XFREE (MTYPE_AS_LIST, aslist);
 }
 
@@ -198,6 +203,7 @@ as_list_insert (const char *name)
   /* Allocate new access_list and copy given name. */
   aslist = as_list_new ();
   aslist->name = strdup (name);
+  assert (aslist->name);
 
   /* If name is made by all digit character.  We treat it as
      number. */
@@ -693,3 +699,28 @@ bgp_filter_init (void)
   install_element (ENABLE_NODE, &show_ip_as_path_access_list_cmd);
   install_element (ENABLE_NODE, &show_ip_as_path_access_list_all_cmd);
 }
+
+void
+bgp_filter_reset (void)
+{
+  struct as_list *aslist;
+  struct as_list *next;
+
+  for (aslist = as_list_master.num.head; aslist; aslist = next)
+    {
+      next = aslist->next;
+      as_list_delete (aslist);
+    }
+
+  for (aslist = as_list_master.str.head; aslist; aslist = next)
+    {
+      next = aslist->next;
+      as_list_delete (aslist);
+    }
+
+  assert (as_list_master.num.head == NULL);
+  assert (as_list_master.num.tail == NULL);
+
+  assert (as_list_master.str.head == NULL);
+  assert (as_list_master.str.tail == NULL);
+}

bgpd/bgp_filter.h

@@ -28,6 +28,7 @@ enum as_filter_type
 };
 
 extern void bgp_filter_init (void);
+extern void bgp_filter_reset (void);
 
 extern enum as_filter_type as_list_apply (struct as_list *, void *);
 

bgpd/bgp_main.c

@@ -31,10 +31,22 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
 #include "log.h"
 #include "privs.h"
 #include "sigevent.h"
+#include "zclient.h"
+#include "routemap.h"
+#include "filter.h"
+#include "plist.h"
 
 #include "bgpd/bgpd.h"
 #include "bgpd/bgp_attr.h"
 #include "bgpd/bgp_mplsvpn.h"
+#include "bgpd/bgp_aspath.h"
+#include "bgpd/bgp_dump.h"
+#include "bgpd/bgp_route.h"
+#include "bgpd/bgp_nexthop.h"
+#include "bgpd/bgp_regex.h"
+#include "bgpd/bgp_clist.h"
+#include "bgpd/bgp_debug.h"
+#include "bgpd/bgp_filter.h"
 
 /* bgpd options, we use GNU getopt library. */
 static const struct option longopts[] = 
@@ -61,6 +73,8 @@ void sighup (void);
 void sigint (void);
 void sigusr1 (void);
 
+static void bgp_exit (int);
+
 static struct quagga_signal_t bgp_signals[] = 
 {
   { 
@@ -182,7 +196,7 @@ sigint (void)
   if (! retain_mode)
     bgp_terminate ();
 
-  exit (0);
+  bgp_exit (0);
 }
 
 /* SIGUSR1 handler. */
@@ -191,6 +205,99 @@ sigusr1 (void)
 {
   zlog_rotate (NULL);
 }
+
+/*
+  Try to free up allocations we know about so that diagnostic tools such as
+  valgrind are able to better illuminate leaks.
+
+  Zebra route removal and protocol teardown are not meant to be done here.
+  For example, "retain_mode" may be set.
+*/
+static void
+bgp_exit (int status)
+{
+  struct bgp *bgp;
+  struct listnode *node, *nnode;
+  int *socket;
+  struct interface *ifp;
+  extern struct zclient *zclient;
+  extern struct zclient *zlookup;
+
+  /* it only makes sense for this to be called on a clean exit */
+  assert (status == 0);
+
+  /* reverse bgp_master_init */
+  for (ALL_LIST_ELEMENTS (bm->bgp, node, nnode, bgp))
+    bgp_delete (bgp);
+  list_free (bm->bgp);
+
+  /* reverse bgp_master_init */
+  for (ALL_LIST_ELEMENTS_RO(bm->listen_sockets, node, socket))
+    {
+      if (close ((int)(long)socket) == -1)
+        zlog_err ("close (%d): %s", (int)(long)socket, safe_strerror (errno));
+    }
+  list_delete (bm->listen_sockets);
+
+  /* reverse bgp_zebra_init/if_init */
+  if (retain_mode)
+    if_add_hook (IF_DELETE_HOOK, NULL);
+  for (ALL_LIST_ELEMENTS (iflist, node, nnode, ifp))
+    if_delete (ifp);
+  list_free (iflist);
+
+  /* reverse bgp_attr_init */
+  bgp_attr_finish ();
+
+  /* reverse bgp_dump_init */
+  bgp_dump_finish ();
+
+  /* reverse bgp_route_init */
+  bgp_route_finish ();
+
+  /* reverse bgp_route_map_init/route_map_init */
+  route_map_finish ();
+
+  /* reverse bgp_scan_init */
+  bgp_scan_finish ();
+
+  /* reverse access_list_init */
+  access_list_add_hook (NULL);
+  access_list_delete_hook (NULL);
+  access_list_reset ();
+
+  /* reverse bgp_filter_init */
+  as_list_add_hook (NULL);
+  as_list_delete_hook (NULL);
+  bgp_filter_reset ();
+
+  /* reverse prefix_list_init */
+  prefix_list_add_hook (NULL);
+  prefix_list_delete_hook (NULL);
+  prefix_list_reset ();
+
+  /* reverse community_list_init */
+  community_list_terminate (bgp_clist);
+
+  cmd_terminate ();
+  vty_terminate ();
+  if (zclient)
+    zclient_free (zclient);
+  if (zlookup)
+    zclient_free (zlookup);
+
+  /* reverse bgp_master_init */
+  if (master)
+    thread_master_free (master);
+
+  if (zlog_default)
+    closezlog (zlog_default);
+
+  if (CONF_BGP_DEBUG (normal, NORMAL))
+    log_memstats_stderr ("bgpd");
+
+  exit (status);
+}
 
 /* Main routine of bgpd. Treatment of argument and start bgp finite
    state machine is handled at here. */

bgpd/bgp_nexthop.c

@@ -65,7 +65,7 @@ static struct bgp_table *cache2_table[AFI_MAX];
 static struct bgp_table *bgp_connected_table[AFI_MAX];
 
 /* BGP nexthop lookup query client. */
-static struct zclient *zlookup = NULL;
+struct zclient *zlookup = NULL;
 
 /* Add nexthop to the end of the list.  */
 static void
@@ -1281,8 +1281,6 @@ bgp_scan_init (void)
 {
   zlookup = zclient_new ();
   zlookup->sock = -1;
-  zlookup->ibuf = stream_new (ZEBRA_MAX_PACKET_SIZ);
-  zlookup->obuf = stream_new (ZEBRA_MAX_PACKET_SIZ);
   zlookup->t_connect = thread_add_event (master, zlookup_connect, zlookup, 0);
 
   bgp_scan_interval = BGP_SCAN_INTERVAL_DEFAULT;
@@ -1314,3 +1312,27 @@ bgp_scan_init (void)
   install_element (RESTRICTED_NODE, &show_ip_bgp_scan_cmd);
   install_element (ENABLE_NODE, &show_ip_bgp_scan_cmd);
 }
+
+void
+bgp_scan_finish (void)
+{
+  bgp_table_unlock (cache1_table[AFI_IP]);
+  cache1_table[AFI_IP] = NULL;
+
+  bgp_table_unlock (cache2_table[AFI_IP]);
+  cache2_table[AFI_IP] = NULL;
+
+  bgp_table_unlock (bgp_connected_table[AFI_IP]);
+  bgp_connected_table[AFI_IP] = NULL;
+
+#ifdef HAVE_IPV6
+  bgp_table_unlock (cache1_table[AFI_IP6]);
+  cache1_table[AFI_IP6] = NULL;
+
+  bgp_table_unlock (cache2_table[AFI_IP6]);
+  cache2_table[AFI_IP6] = NULL;
+
+  bgp_table_unlock (bgp_connected_table[AFI_IP6]);
+  bgp_connected_table[AFI_IP6] = NULL;
+#endif /* HAVE_IPV6 */
+}

bgpd/bgp_nexthop.h

@@ -47,6 +47,7 @@ struct bgp_nexthop_cache
 };
 
 extern void bgp_scan_init (void);
+extern void bgp_scan_finish (void);
 extern int bgp_nexthop_lookup (afi_t, struct peer *peer, struct bgp_info *,
 			int *, int *);
 extern void bgp_connected_add (struct connected *c);

bgpd/bgp_packet.c

@@ -2193,7 +2193,7 @@ bgp_capability_msg_parse (struct peer *peer, u_char *pnt, bgp_size_t length)
               peer->afc_nego[afi][safi] = 0;
 
               if (peer_active_nego (peer))
-                bgp_clear_route (peer, afi, safi);
+                bgp_clear_route (peer, afi, safi, BGP_CLEAR_ROUTE_NORMAL);
               else
                 BGP_EVENT_ADD (peer, BGP_Stop);
             }

bgpd/bgp_route.c

@@ -1464,11 +1464,9 @@ bgp_process_rsclient (struct work_queue *wq, void *data)
   struct bgp_info *new_select;
   struct bgp_info *old_select;
   struct bgp_info_pair old_and_new;
-  struct attr attr;
   struct listnode *node, *nnode;
   struct peer *rsclient = rn->table->owner;
   
-  memset (&attr, 0, sizeof (struct attr));
   /* Best path selection. */
   bgp_best_selection (bgp, rn, &old_and_new);
   new_select = old_and_new.new;
@@ -1476,23 +1474,25 @@ bgp_process_rsclient (struct work_queue *wq, void *data)
 
   if (CHECK_FLAG (rsclient->sflags, PEER_STATUS_GROUP))
     {
-      for (ALL_LIST_ELEMENTS (rsclient->group->peer, node, nnode, rsclient))
-	{
-	  /* Nothing to do. */
-	  if (old_select && old_select == new_select)
-	    if (!CHECK_FLAG (old_select->flags, BGP_INFO_ATTR_CHANGED))
-	      continue;
-
-	  if (old_select)
-	    bgp_info_unset_flag (rn, old_select, BGP_INFO_SELECTED);
-	  if (new_select)
-	    {
-	      bgp_info_set_flag (rn, new_select, BGP_INFO_SELECTED);
-	      bgp_info_unset_flag (rn, new_select, BGP_INFO_ATTR_CHANGED);
-	    }
-
-	  bgp_process_announce_selected (rsclient, new_select, rn, afi, safi);
-	}
+      if (rsclient->group)
+        for (ALL_LIST_ELEMENTS (rsclient->group->peer, node, nnode, rsclient))
+          {
+            /* Nothing to do. */
+            if (old_select && old_select == new_select)
+              if (!CHECK_FLAG (old_select->flags, BGP_INFO_ATTR_CHANGED))
+                continue;
+
+            if (old_select)
+              bgp_info_unset_flag (rn, old_select, BGP_INFO_SELECTED);
+            if (new_select)
+              {
+                bgp_info_set_flag (rn, new_select, BGP_INFO_SELECTED);
+                bgp_info_unset_flag (rn, new_select, BGP_INFO_ATTR_CHANGED);
+              }
+
+            bgp_process_announce_selected (rsclient, new_select, rn,
+                                           afi, safi);
+          }
     }
   else
     {
@@ -1509,8 +1509,6 @@ bgp_process_rsclient (struct work_queue *wq, void *data)
   if (old_select && CHECK_FLAG (old_select->flags, BGP_INFO_REMOVED))
     bgp_info_reap (rn, old_select);
   
-  bgp_attr_extra_free (&attr);
-  
   UNSET_FLAG (rn->flags, BGP_NODE_PROCESS_SCHEDULED);
   return WQ_SUCCESS;
 }
@@ -1593,9 +1591,11 @@ static void
 bgp_processq_del (struct work_queue *wq, void *data)
 {
   struct bgp_process_queue *pq = data;
+  struct bgp_table *table = pq->rn->table;
   
-  bgp_unlock(pq->bgp);
+  bgp_unlock (pq->bgp);
   bgp_unlock_node (pq->rn);
+  bgp_table_unlock (table);
   XFREE (MTYPE_BGP_PROCESS_QUEUE, pq);
 }
 
@@ -1641,10 +1641,12 @@ bgp_process (struct bgp *bgp, struct bgp_node *rn, afi_t afi, safi_t safi)
                     sizeof (struct bgp_process_queue));
   if (!pqnode)
     return;
-  
-  pqnode->rn = bgp_lock_node (rn); /* unlocked by bgp_processq_del */
+
+  /* all unlocked in bgp_processq_del */
+  bgp_table_lock (rn->table);
+  pqnode->rn = bgp_lock_node (rn);
   pqnode->bgp = bgp;
-  bgp_lock(bgp);
+  bgp_lock (bgp);
   pqnode->afi = afi;
   pqnode->safi = safi;
   
@@ -1805,8 +1807,6 @@ bgp_update_rsclient (struct peer *rsclient, afi_t afi, safi_t safi,
   const char *reason;
   char buf[SU_ADDRSTRLEN];
 
-  //memset (new_attr, 0, sizeof (struct attr));
-  
   /* Do not insert announces from a rsclient into its own 'bgp_table'. */
   if (peer == rsclient)
     return;
@@ -1894,10 +1894,10 @@ bgp_update_rsclient (struct peer *rsclient, afi_t afi, safi_t safi,
                     inet_ntop(p->family, &p->u.prefix, buf, SU_ADDRSTRLEN),
                     p->prefixlen, rsclient->host);
 
-                    bgp_unlock_node (rn);
-                    bgp_attr_unintern (attr_new);
+          bgp_unlock_node (rn);
+          bgp_attr_unintern (attr_new);
 
-                    return;
+          return;
         }
 
       /* Withdraw/Announce before we fully processed the withdraw */
@@ -1992,13 +1992,13 @@ static void
 bgp_withdraw_rsclient (struct peer *rsclient, afi_t afi, safi_t safi,
       struct peer *peer, struct prefix *p, int type, int sub_type,
       struct prefix_rd *prd, u_char *tag)
-    {
+{
   struct bgp_node *rn;
   struct bgp_info *ri;
   char buf[SU_ADDRSTRLEN];
 
   if (rsclient == peer)
-       return;
+    return;
 
   rn = bgp_afi_node_get (rsclient->rib[afi][safi], afi, safi, p, prd);
 
@@ -2017,8 +2017,8 @@ bgp_withdraw_rsclient (struct peer *rsclient, afi_t afi, safi_t safi,
           p->prefixlen);
 
   /* Unlock bgp_node_get() lock. */
-      bgp_unlock_node (rn);
-    }
+  bgp_unlock_node (rn);
+}
 
 static int
 bgp_update_main (struct peer *peer, struct prefix *p, struct attr *attr,
@@ -2432,7 +2432,7 @@ void
 bgp_default_originate (struct peer *peer, afi_t afi, safi_t safi, int withdraw)
 {
   struct bgp *bgp;
-  struct attr attr;
+  struct attr attr = { 0 };
   struct aspath *aspath = { 0 };
   struct prefix p;
   struct bgp_info binfo;
@@ -2521,9 +2521,7 @@ bgp_announce_table (struct peer *peer, afi_t afi, safi_t safi,
 {
   struct bgp_node *rn;
   struct bgp_info *ri;
-  struct attr attr;
-  
-  memset (&attr, 0, sizeof (struct attr));
+  struct attr attr = { 0 };
   
   if (! table)
     table = (rsclient) ? peer->rib[afi][safi] : peer->bgp->rib[afi][safi];
@@ -2667,10 +2665,18 @@ bgp_soft_reconfig_in (struct peer *peer, afi_t afi, safi_t safi)
 	bgp_soft_reconfig_table (peer, afi, safi, table);
 }
 
+
+struct bgp_clear_node_queue
+{
+  struct bgp_node *rn;
+  enum bgp_clear_route_type purpose;
+};
+
 static wq_item_status
 bgp_clear_route_node (struct work_queue *wq, void *data)
 {
-  struct bgp_node *rn = data;
+  struct bgp_clear_node_queue *cnq = data;
+  struct bgp_node *rn = cnq->rn;
   struct peer *peer = wq->spec.data;
   struct bgp_info *ri;
   afi_t afi = rn->table->afi;
@@ -2679,7 +2685,7 @@ bgp_clear_route_node (struct work_queue *wq, void *data)
   assert (rn && peer);
   
   for (ri = rn->info; ri; ri = ri->next)
-    if (ri->peer == peer)
+    if (ri->peer == peer || cnq->purpose == BGP_CLEAR_ROUTE_MY_RSCLIENT)
       {
         /* graceful restart STALE flag set. */
         if (CHECK_FLAG (peer->sflags, PEER_STATUS_NSF_WAIT)
@@ -2697,9 +2703,13 @@ bgp_clear_route_node (struct work_queue *wq, void *data)
 static void
 bgp_clear_node_queue_del (struct work_queue *wq, void *data)
 {
-  struct bgp_node *rn = data;
+  struct bgp_clear_node_queue *cnq = data;
+  struct bgp_node *rn = cnq->rn;
+  struct bgp_table *table = rn->table;
   
   bgp_unlock_node (rn); 
+  bgp_table_unlock (table);
+  XFREE (MTYPE_BGP_CLEAR_NODE_QUEUE, cnq);
 }
 
 static void
@@ -2707,10 +2717,10 @@ bgp_clear_node_complete (struct work_queue *wq)
 {
   struct peer *peer = wq->spec.data;
   
-  peer_unlock (peer); /* bgp_clear_node_complete */
-  
   /* Tickle FSM to start moving again */
   BGP_EVENT_ADD (peer, Clearing_Completed);
+
+  peer_unlock (peer); /* bgp_clear_route */
 }
 
 static void
@@ -2739,7 +2749,8 @@ bgp_clear_node_queue_init (struct peer *peer)
 
 static void
 bgp_clear_route_table (struct peer *peer, afi_t afi, safi_t safi,
-                      struct bgp_table *table, struct peer *rsclient)
+                       struct bgp_table *table, struct peer *rsclient,
+                       enum bgp_clear_route_type purpose)
 {
   struct bgp_node *rn;
   
@@ -2792,21 +2803,30 @@ bgp_clear_route_table (struct peer *peer, afi_t afi, safi_t safi,
        * problem at this time,
        */
       for (ri = rn->info; ri; ri = ri->next)
-        if (ri->peer == peer)
+        if (ri->peer == peer || purpose == BGP_CLEAR_ROUTE_MY_RSCLIENT)
           {
-            bgp_lock_node (rn); /* unlocked: bgp_clear_node_queue_del */
-            work_queue_add (peer->clear_node_queue, rn);
+            struct bgp_clear_node_queue *cnq;
+
+            /* both unlocked in bgp_clear_node_queue_del */
+            bgp_table_lock (rn->table);
+            bgp_lock_node (rn);
+            cnq = XCALLOC (MTYPE_BGP_CLEAR_NODE_QUEUE,
+                           sizeof (struct bgp_clear_node_queue));
+            cnq->rn = rn;
+            cnq->purpose = purpose;
+            work_queue_add (peer->clear_node_queue, cnq);
+            break;
           }
 
       for (ain = rn->adj_in; ain; ain = ain->next)
-        if (ain->peer == peer)
+        if (ain->peer == peer || purpose == BGP_CLEAR_ROUTE_MY_RSCLIENT)
           {
             bgp_adj_in_remove (rn, ain);
             bgp_unlock_node (rn);
             break;
           }
       for (aout = rn->adj_out; aout; aout = aout->next)
-        if (aout->peer == peer)
+        if (aout->peer == peer || purpose == BGP_CLEAR_ROUTE_MY_RSCLIENT)
           {
             bgp_adj_out_remove (rn, aout, peer, afi, safi);
             bgp_unlock_node (rn);
@@ -2817,7 +2837,8 @@ bgp_clear_route_table (struct peer *peer, afi_t afi, safi_t safi,
 }
 
 void
-bgp_clear_route (struct peer *peer, afi_t afi, safi_t safi)
+bgp_clear_route (struct peer *peer, afi_t afi, safi_t safi,
+                 enum bgp_clear_route_type purpose)
 {
   struct bgp_node *rn;
   struct bgp_table *table;
@@ -2841,19 +2862,31 @@ bgp_clear_route (struct peer *peer, afi_t afi, safi_t safi)
    */
   if (!peer->clear_node_queue->thread)
     peer_lock (peer); /* bgp_clear_node_complete */
-  
-  if (safi != SAFI_MPLS_VPN)
-    bgp_clear_route_table (peer, afi, safi, NULL, NULL);
-  else
-    for (rn = bgp_table_top (peer->bgp->rib[afi][safi]); rn;
-	 rn = bgp_route_next (rn))
-      if ((table = rn->info) != NULL)
-       bgp_clear_route_table (peer, afi, safi, table, NULL);
 
-  for (ALL_LIST_ELEMENTS (peer->bgp->rsclient, node, nnode, rsclient))
+  switch (purpose)
     {
-      if (CHECK_FLAG(rsclient->af_flags[afi][safi], PEER_FLAG_RSERVER_CLIENT))
-        bgp_clear_route_table (peer, afi, safi, NULL, rsclient);
+    case BGP_CLEAR_ROUTE_NORMAL:
+      if (safi != SAFI_MPLS_VPN)
+        bgp_clear_route_table (peer, afi, safi, NULL, NULL, purpose);
+      else
+        for (rn = bgp_table_top (peer->bgp->rib[afi][safi]); rn;
+             rn = bgp_route_next (rn))
+          if ((table = rn->info) != NULL)
+            bgp_clear_route_table (peer, afi, safi, table, NULL, purpose);
+
+      for (ALL_LIST_ELEMENTS (peer->bgp->rsclient, node, nnode, rsclient))
+        if (CHECK_FLAG(rsclient->af_flags[afi][safi],
+                       PEER_FLAG_RSERVER_CLIENT))
+          bgp_clear_route_table (peer, afi, safi, NULL, rsclient, purpose);
+      break;
+
+    case BGP_CLEAR_ROUTE_MY_RSCLIENT:
+      bgp_clear_route_table (peer, afi, safi, NULL, peer, purpose);
+      break;
+
+    default:
+      assert (0);
+      break;
     }
   
   /* If no routes were cleared, nothing was added to workqueue, the
@@ -2887,7 +2920,7 @@ bgp_clear_route_all (struct peer *peer)
 
   for (afi = AFI_IP; afi < AFI_MAX; afi++)
     for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++)
-      bgp_clear_route (peer, afi, safi);
+      bgp_clear_route (peer, afi, safi, BGP_CLEAR_ROUTE_NORMAL);
 }
 
 void
@@ -12276,3 +12309,10 @@ bgp_route_init (void)
   install_element (BGP_IPV4_NODE, &bgp_damp_unset_cmd);
   install_element (BGP_IPV4_NODE, &bgp_damp_unset2_cmd);
 }
+
+void
+bgp_route_finish (void)
+{
+  bgp_table_unlock (bgp_distance_table);
+  bgp_distance_table = NULL;
+}

bgpd/bgp_route.h

@@ -61,7 +61,7 @@ struct bgp_info
   time_t uptime;
 
   /* reference count */
-  unsigned int lock;
+  int lock;
   
   /* BGP information status.  */
   u_int16_t flags;
@@ -160,8 +160,15 @@ struct bgp_static
 #define UNSUPPRESS_MAP_NAME(F)  ((F)->usmap.name)
 #define UNSUPPRESS_MAP(F)       ((F)->usmap.map)
 
+enum bgp_clear_route_type
+{
+  BGP_CLEAR_ROUTE_NORMAL,
+  BGP_CLEAR_ROUTE_MY_RSCLIENT
+};
+
 /* Prototypes. */
 extern void bgp_route_init (void);
+extern void bgp_route_finish (void);
 extern void bgp_cleanup_routes (void);
 extern void bgp_announce_route (struct peer *, afi_t, safi_t);
 extern void bgp_announce_route_all (struct peer *);
@@ -169,7 +176,8 @@ extern void bgp_default_originate (struct peer *, afi_t, safi_t, int);
 extern void bgp_soft_reconfig_in (struct peer *, afi_t, safi_t);
 extern void bgp_soft_reconfig_rsclient (struct peer *, afi_t, safi_t);
 extern void bgp_check_local_routes_rsclient (struct peer *rsclient, afi_t afi, safi_t safi);
-extern void bgp_clear_route (struct peer *, afi_t, safi_t);
+extern void bgp_clear_route (struct peer *, afi_t, safi_t,
+                             enum bgp_clear_route_type);
 extern void bgp_clear_route_all (struct peer *);
 extern void bgp_clear_adj_in (struct peer *, afi_t, safi_t);
 extern void bgp_clear_stale_route (struct peer *, afi_t, safi_t);

bgpd/bgp_table.c

@@ -38,6 +38,7 @@ bgp_table_init (afi_t afi, safi_t safi)
 
   rt = XCALLOC (MTYPE_BGP_TABLE, sizeof (struct bgp_table));
 
+  bgp_table_lock(rt);
   rt->type = BGP_TABLE_MAIN;
   rt->afi = afi;
   rt->safi = safi;
@@ -46,10 +47,29 @@ bgp_table_init (afi_t afi, safi_t safi)
 }
 
 void
+bgp_table_lock (struct bgp_table *rt)
+{
+  rt->lock++;
+}
+
+void
+bgp_table_unlock (struct bgp_table *rt)
+{
+  assert (rt->lock > 0);
+  rt->lock--;
+
+  if (rt->lock == 0)
+    bgp_table_free (rt);
+}
+
+void
 bgp_table_finish (struct bgp_table **rt)
 {
-  bgp_table_free (*rt);
-  *rt = NULL;
+  if (*rt != NULL)
+    {
+      bgp_table_unlock(*rt);
+      *rt = NULL;
+    }
 }
 
 static struct bgp_node *
@@ -91,6 +111,9 @@ bgp_table_free (struct bgp_table *rt)
 
   node = rt->top;
 
+  /* Bulk deletion of nodes remaining in this table.  This function is not
+     called until workers have completed their dependency on this table.
+     A final bgp_unlock_node() will not be called for these nodes. */
   while (node)
     {
       if (node->l_left)
@@ -108,22 +131,31 @@ bgp_table_free (struct bgp_table *rt)
       tmp_node = node;
       node = node->parent;
 
+      tmp_node->table->count--;
+      tmp_node->lock = 0;  /* to cause assert if unlocked after this */
+      bgp_node_free (tmp_node);
+
       if (node != NULL)
 	{
 	  if (node->l_left == tmp_node)
 	    node->l_left = NULL;
 	  else
 	    node->l_right = NULL;
-
-	  bgp_node_free (tmp_node);
 	}
       else
 	{
-	  bgp_node_free (tmp_node);
 	  break;
 	}
     }
  
+  assert (rt->count == 0);
+
+  if (rt->owner)
+    {
+      peer_unlock (rt->owner);
+      rt->owner = NULL;
+    }
+
   XFREE (MTYPE_BGP_TABLE, rt);
   return;
 }
@@ -217,6 +249,7 @@ bgp_lock_node (struct bgp_node *node)
 void
 bgp_unlock_node (struct bgp_node *node)
 {
+  assert (node->lock > 0);
   node->lock--;
 
   if (node->lock == 0)
@@ -344,6 +377,7 @@ bgp_node_get (struct bgp_table *const table, struct prefix *p)
       if (new->p.prefixlen != p->prefixlen)
 	{
 	  match = new;
+	  bgp_lock_node (match);
 	  new = bgp_node_set (table, p);
 	  set_link (match, new);
 	  table->count++;

bgpd/bgp_table.h

@@ -35,8 +35,10 @@ struct bgp_table
   afi_t afi;
   safi_t safi;
   
+  int lock;
+
   /* The owner of this 'bgp_table' structure. */
-  void *owner;
+  struct peer *owner;
 
   struct bgp_node *top;
   
@@ -61,13 +63,15 @@ struct bgp_node
 
   struct bgp_node *prn;
 
-  unsigned int lock;
+  int lock;
 
   u_char flags;
 #define BGP_NODE_PROCESS_SCHEDULED	(1 << 0)
 };
 
 extern struct bgp_table *bgp_table_init (afi_t, safi_t);
+extern void bgp_table_lock (struct bgp_table *);
+extern void bgp_table_unlock (struct bgp_table *);
 extern void bgp_table_finish (struct bgp_table **);
 extern void bgp_unlock_node (struct bgp_node *node);
 extern struct bgp_node *bgp_table_top (const struct bgp_table *const);

bgpd/bgp_vty.c

@@ -2074,6 +2074,7 @@ peer_rsclient_set_vty (struct vty *vty, const char *peer_str,
   struct listnode *node, *nnode;
   struct bgp_filter *pfilter;
   struct bgp_filter *gfilter;
+  int locked_and_added = 0;
 
   bgp = vty->index;
 
@@ -2089,15 +2090,25 @@ peer_rsclient_set_vty (struct vty *vty, const char *peer_str,
     {
       peer = peer_lock (peer); /* rsclient peer list reference */
       listnode_add_sort (bgp->rsclient, peer);
+      locked_and_added = 1;
     }
 
   ret = peer_af_flag_set (peer, afi, safi, PEER_FLAG_RSERVER_CLIENT);
   if (ret < 0)
-    return bgp_vty_return (vty, ret);
+    {
+      if (locked_and_added)
+        {
+          listnode_delete (bgp->rsclient, peer);
+          peer_unlock (peer); /* rsclient peer list reference */
+        }
+
+      return bgp_vty_return (vty, ret);
+    }
 
   peer->rib[afi][safi] = bgp_table_init (afi, safi);
   peer->rib[afi][safi]->type = BGP_TABLE_RSCLIENT;
-  peer->rib[afi][safi]->owner = peer;
+  /* RIB peer reference.  Released when table is free'd in bgp_table_free. */
+  peer->rib[afi][safi]->owner = peer_lock (peer);
 
   /* Check for existing 'network' and 'redistribute' routes. */
   bgp_check_local_routes_rsclient (peer, afi, safi);
@@ -2190,8 +2201,9 @@ peer_rsclient_unset_vty (struct vty *vty, const char *peer_str,
 
   if ( ! peer_rsclient_active (peer) )
     {
-      peer_unlock (peer); /* peer bgp rsclient reference */
+      bgp_clear_route (peer, afi, safi, BGP_CLEAR_ROUTE_MY_RSCLIENT);
       listnode_delete (bgp->rsclient, peer);
+      peer_unlock (peer); /* peer bgp rsclient reference */
     }
 
   bgp_table_finish (&peer->rib[bgp_node_afi(vty)][bgp_node_safi(vty)]);

bgpd/bgp_zebra.c

@@ -39,7 +39,7 @@ Boston, MA 02111-1307, USA.  */
 #include "bgpd/bgp_debug.h"
 
 /* All information about zebra. */
-static struct zclient *zclient = NULL;
+struct zclient *zclient = NULL;
 struct in_addr router_id_zebra;
 
 /* Router-id update message from zebra. */

bgpd/bgpd.c

@@ -727,7 +727,6 @@ struct peer *
 peer_lock (struct peer *peer)
 {
   assert (peer && (peer->lock >= 0));
-  assert (peer->status != Deleted);
     
   peer->lock++;
   
@@ -1109,7 +1108,7 @@ peer_deactivate (struct peer *peer, afi_t afi, safi_t safi)
 		  bgp_capability_send (peer, afi, safi,
 				       CAPABILITY_CODE_MP,
 				       CAPABILITY_ACTION_UNSET);
-		  bgp_clear_route (peer, afi, safi);
+		  bgp_clear_route (peer, afi, safi, BGP_CLEAR_ROUTE_NORMAL);
 		  peer->pcount[afi][safi] = 0;
 		}
 	      else
@@ -1177,6 +1176,7 @@ peer_delete (struct peer *peer)
   safi_t safi;
   struct bgp *bgp;
   struct bgp_filter *filter;
+  struct listnode *pn;
 
   assert (peer->status != Deleted);
   
@@ -1185,12 +1185,10 @@ peer_delete (struct peer *peer)
   if (CHECK_FLAG (peer->sflags, PEER_STATUS_NSF_WAIT))
     peer_nsf_stop (peer);
 
-  /* If this peer belongs to peer group.  Clearn up the
+  /* If this peer belongs to peer group, clear up the
      relationship.  */
   if (peer->group)
     {
-      struct listnode *pn;
-
       if ((pn = listnode_lookup (peer->group->peer, peer)))
         {
           peer = peer_unlock (peer); /* group->peer list reference */
@@ -1220,22 +1218,25 @@ peer_delete (struct peer *peer)
   bgp_timer_set (peer); /* stops all timers for Deleted */
   
   /* Delete from all peer list. */
-  if (! CHECK_FLAG (peer->sflags, PEER_STATUS_GROUP))
+  if (! CHECK_FLAG (peer->sflags, PEER_STATUS_GROUP)
+      && (pn = listnode_lookup (bgp->peer, peer)))
     {
-      struct listnode *pn;
-      
-      if ((pn = listnode_lookup (bgp->peer, peer)))
-        {
-          peer_unlock (peer); /* bgp peer list reference */
-          list_delete_node (bgp->peer, pn);
-        }
+      peer_unlock (peer); /* bgp peer list reference */
+      list_delete_node (bgp->peer, pn);
+    }
       
-      if (peer_rsclient_active (peer)
-          && (pn = listnode_lookup (bgp->rsclient, peer)))
-        {
-          peer_unlock (peer); /* rsclient list reference */
-          list_delete_node (bgp->rsclient, pn);
-        }
+  if (peer_rsclient_active (peer)
+      && (pn = listnode_lookup (bgp->rsclient, peer)))
+    {
+      peer_unlock (peer); /* rsclient list reference */
+      list_delete_node (bgp->rsclient, pn);
+
+      /* Clear our own rsclient ribs. */
+      for (afi = AFI_IP; afi < AFI_MAX; afi++)
+        for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++)
+          if (CHECK_FLAG(peer->af_flags[afi][safi],
+                         PEER_FLAG_RSERVER_CLIENT))
+            bgp_clear_route (peer, afi, safi, BGP_CLEAR_ROUTE_MY_RSCLIENT);
     }
 
   /* Free RIB for any family in which peer is RSERVER_CLIENT, and is not
@@ -1366,7 +1367,7 @@ peer_group_get (struct bgp *bgp, const char *name)
   group->conf = peer_new (bgp);
   if (! bgp_flag_check (bgp, BGP_FLAG_NO_DEFAULT_IPV4))
     group->conf->afc[AFI_IP][SAFI_UNICAST] = 1;
-  group->conf->host = strdup (name);
+  group->conf->host = XSTRDUP (MTYPE_BGP_PEER_HOST, name);
   group->conf->group = group;
   group->conf->as = 0; 
   group->conf->ttl = 1;
@@ -1822,6 +1823,9 @@ peer_group_bind (struct bgp *bgp, union sockunion *su,
         {
           peer_unlock (peer); /* peer rsclient reference */
           list_delete_node (bgp->rsclient, pn);
+
+          /* Clear our own rsclient rib for this afi/safi. */
+          bgp_clear_route (peer, afi, safi, BGP_CLEAR_ROUTE_MY_RSCLIENT);
         }
 
       bgp_table_finish (&peer->rib[afi][safi]);
@@ -1914,7 +1918,7 @@ bgp_create (as_t *as, const char *name)
   
   bgp_lock (bgp);
   bgp->peer_self = peer_new (bgp);
-  bgp->peer_self->host = strdup ("Static announcement");
+  bgp->peer_self->host = XSTRDUP (MTYPE_BGP_PEER_HOST, "Static announcement");
 
   bgp->peer = list_new ();
   bgp->peer->cmp = (int (*)(void *, void *)) peer_cmp;
@@ -2060,14 +2064,13 @@ bgp_delete (struct bgp *bgp)
       if (i != ZEBRA_ROUTE_BGP)
 	bgp_redistribute_unset (bgp, afi, i);
 
-  for (ALL_LIST_ELEMENTS (bgp->group, node, next, group))
-    peer_group_delete (group);
-
   for (ALL_LIST_ELEMENTS (bgp->peer, node, next, peer))
     peer_delete (peer);
 
-  for (ALL_LIST_ELEMENTS (bgp->rsclient, node, next, peer))
-    peer_delete (peer);
+  for (ALL_LIST_ELEMENTS (bgp->group, node, next, group))
+    peer_group_delete (group);
+
+  assert (listcount (bgp->rsclient) == 0);
 
   if (bgp->peer_self) {
     peer_delete(bgp->peer_self);
@@ -2095,6 +2098,7 @@ bgp_lock (struct bgp *bgp)
 void
 bgp_unlock(struct bgp *bgp)
 {
+  assert(bgp->lock > 0);
   if (--bgp->lock == 0)
     bgp_free (bgp);
 }
@@ -2116,11 +2120,11 @@ bgp_free (struct bgp *bgp)
     for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++)
       {
 	if (bgp->route[afi][safi])
-	  XFREE (MTYPE_ROUTE_TABLE, bgp->route[afi][safi]);
+          bgp_table_finish (&bgp->route[afi][safi]);
 	if (bgp->aggregate[afi][safi])
-	  XFREE (MTYPE_ROUTE_TABLE,bgp->aggregate[afi][safi]) ;
+          bgp_table_finish (&bgp->aggregate[afi][safi]) ;
 	if (bgp->rib[afi][safi])
-	  XFREE (MTYPE_ROUTE_TABLE,bgp->rib[afi][safi]);
+          bgp_table_finish (&bgp->rib[afi][safi]);
       }
   XFREE (MTYPE_BGP, bgp);
 }

lib/command.c

@@ -37,6 +37,9 @@ Boston, MA 02111-1307, USA.  */
    each daemon maintains each own cmdvec. */
 vector cmdvec = NULL;
 
+struct desc desc_cr;
+char *command_cr = NULL;
+
 /* Host information structure. */
 struct host host;
 
@@ -199,8 +202,8 @@ install_node (struct cmd_node *node,
 static int
 cmp_node (const void *p, const void *q)
 {
-  const struct cmd_element *a = *(struct cmd_element **)p;
-  const struct cmd_element *b = *(struct cmd_element **)q;
+  const struct cmd_element *a = *(struct cmd_element * const *)p;
+  const struct cmd_element *b = *(struct cmd_element * const *)q;
 
   return strcmp (a->string, b->string);
 }
@@ -208,8 +211,8 @@ cmp_node (const void *p, const void *q)
 static int
 cmp_desc (const void *p, const void *q)
 {
-  const struct desc *a = *(struct desc **)p;
-  const struct desc *b = *(struct desc **)q;
+  const struct desc *a = *(struct desc * const *)p;
+  const struct desc *b = *(struct desc * const *)q;
 
   return strcmp (a->cmd, b->cmd);
 }
@@ -223,7 +226,7 @@ sort_node ()
   vector descvec;
   struct cmd_element *cmd_element;
 
-  for (i = 0; i < vector_active (cmdvec); i++) 
+  for (i = 0; i < vector_active (cmdvec); i++)
     if ((cnode = vector_slot (cmdvec, i)) != NULL)
       {	
 	vector cmd_vector = cnode->cmd_vector;
@@ -497,7 +500,9 @@ install_element (enum node_type ntype, struct cmd_element *cmd)
 
   vector_set (cnode->cmd_vector, cmd);
 
-  cmd->strvec = cmd_make_descvec (cmd->string, cmd->doc);
+  if (cmd->strvec == NULL)
+    cmd->strvec = cmd_make_descvec (cmd->string, cmd->doc);
+
   cmd->cmdsize = cmd_cmdsize (cmd->strvec);
 }
 
@@ -1588,7 +1593,6 @@ cmd_describe_command_real (vector vline, struct vty *vty, int *status)
   int ret;
   enum match_type match;
   char *command;
-  static struct desc desc_cr = { "<cr>", "" };
 
   /* Set index. */
   if (vector_active (vline) == 0)
@@ -1665,7 +1669,6 @@ cmd_describe_command_real (vector vline, struct vty *vty, int *status)
   for (i = 0; i < vector_active (cmd_vector); i++)
     if ((cmd_element = vector_slot (cmd_vector, i)) != NULL)
       {
-	const char *string = NULL;
 	vector strvec = cmd_element->strvec;
 
 	/* if command is NULL, index may be equal to vector_active */
@@ -1676,8 +1679,7 @@ cmd_describe_command_real (vector vline, struct vty *vty, int *status)
 	    /* Check if command is completed. */
 	    if (command == NULL && index == vector_active (strvec))
 	      {
-		string = "<cr>";
-		if (!desc_unique_string (matchvec, string))
+		if (!desc_unique_string (matchvec, command_cr))
 		  vector_set (matchvec, &desc_cr);
 	      }
 	    else
@@ -1689,6 +1691,8 @@ cmd_describe_command_real (vector vline, struct vty *vty, int *status)
 		for (j = 0; j < vector_active (descvec); j++)
 		  if ((desc = vector_slot (descvec, j)))
 		    {
+		      const char *string;
+
 		      string = cmd_entry_function_desc (command, desc->cmd);
 		      if (string)
 			{
@@ -3506,6 +3510,8 @@ DEFUN (no_banner_motd,
 void
 host_config_set (char *filename)
 {
+  if (host.config)
+    XFREE (MTYPE_HOST, host.config);
   host.config = XSTRDUP (MTYPE_HOST, filename);
 }
 
@@ -3529,6 +3535,10 @@ install_default (enum node_type node)
 void
 cmd_init (int terminal)
 {
+  command_cr = XSTRDUP(MTYPE_STRVEC, "<cr>");
+  desc_cr.cmd = command_cr;
+  desc_cr.str = XSTRDUP(MTYPE_STRVEC, "");
+
   /* Allocate initial top vector of commands. */
   cmdvec = vector_init (VECTOR_MIN_SIZE);
 
@@ -3645,3 +3655,74 @@ cmd_init (int terminal)
     }
   srand(time(NULL));
 }
+
+void
+cmd_terminate ()
+{
+  unsigned int i, j, k, l;
+  struct cmd_node *cmd_node;
+  struct cmd_element *cmd_element;
+  struct desc *desc;
+  vector cmd_node_v, cmd_element_v, desc_v;
+
+  if (cmdvec)
+    {
+      for (i = 0; i < vector_active (cmdvec); i++) 
+        if ((cmd_node = vector_slot (cmdvec, i)) != NULL)
+          {
+            cmd_node_v = cmd_node->cmd_vector;
+
+            for (j = 0; j < vector_active (cmd_node_v); j++)
+              if ((cmd_element = vector_slot (cmd_node_v, j)) != NULL &&
+                  cmd_element->strvec != NULL)
+                {
+                  cmd_element_v = cmd_element->strvec;
+
+                  for (k = 0; k < vector_active (cmd_element_v); k++)
+                    if ((desc_v = vector_slot (cmd_element_v, k)) != NULL)
+                      {
+                        for (l = 0; l < vector_active (desc_v); l++)
+                          if ((desc = vector_slot (desc_v, l)) != NULL)
+                            {
+                              if (desc->cmd)
+                                XFREE (MTYPE_STRVEC, desc->cmd);
+                              if (desc->str)
+                                XFREE (MTYPE_STRVEC, desc->str);
+
+                              XFREE (MTYPE_DESC, desc);
+                            }
+                        vector_free (desc_v);
+                      }
+
+                  cmd_element->strvec = NULL;
+                  vector_free (cmd_element_v);
+                }
+
+            vector_free (cmd_node_v);
+          }
+
+      vector_free (cmdvec);
+      cmdvec = NULL;
+    }
+
+  if (command_cr)
+    XFREE(MTYPE_STRVEC, command_cr);
+  if (desc_cr.str)
+    XFREE(MTYPE_STRVEC, desc_cr.str);
+  if (host.name)
+    XFREE (MTYPE_HOST, host.name);
+  if (host.password)
+    XFREE (MTYPE_HOST, host.password);
+  if (host.password_encrypt)
+    XFREE (MTYPE_HOST, host.password_encrypt);
+  if (host.enable)
+    XFREE (MTYPE_HOST, host.enable);
+  if (host.enable_encrypt)
+    XFREE (MTYPE_HOST, host.enable_encrypt);
+  if (host.logfile)
+    XFREE (MTYPE_HOST, host.logfile);
+  if (host.motdfile)
+    XFREE (MTYPE_HOST, host.motdfile);
+  if (host.config)
+    XFREE (MTYPE_HOST, host.config);
+}

lib/command.h

@@ -147,8 +147,8 @@ struct cmd_element
 /* Command description structure. */
 struct desc
 {
-  const char *cmd;			/* Command string. */
-  const char *str;			/* Command's description. */
+  char *cmd;                    /* Command string. */
+  char *str;                    /* Command's description. */
 };
 
 /* Return value of the commands. */
@@ -347,6 +347,7 @@ extern int cmd_execute_command (vector, struct vty *, struct cmd_element **, int
 extern int cmd_execute_command_strict (vector, struct vty *, struct cmd_element **);
 extern void config_replace_string (struct cmd_element *, char *, ...);
 extern void cmd_init (int);
+extern void cmd_terminate (void);
 
 /* Export typical functions. */
 extern struct cmd_element config_end_cmd;
@@ -361,4 +362,7 @@ extern void print_version (const char *);
 
 /* struct host global, ick */
 extern struct host host; 
+
+/* "<cr>" global */
+extern char *command_cr;
 #endif /* _ZEBRA_COMMAND_H */

lib/log.c

@@ -649,7 +649,9 @@ void
 closezlog (struct zlog *zl)
 {
   closelog();
-  fclose (zl->fp);
+
+  if (zl->fp != NULL)
+    fclose (zl->fp);
 
   XFREE (MTYPE_ZLOG, zl);
 }

lib/memory.c

@@ -127,7 +127,7 @@ zstrdup (int type, const char *str)
 static struct 
 {
   const char *name;
-  unsigned long alloc;
+  long alloc;
   unsigned long t_malloc;
   unsigned long c_malloc;
   unsigned long t_calloc;
@@ -214,9 +214,9 @@ mtype_zstrdup (const char *file, int line, int type, const char *str)
 static struct 
 {
   char *name;
-  unsigned long alloc;
+  long alloc;
 } mstat [MTYPE_MAX];
-#endif /* MTPYE_LOG */
+#endif /* MEMORY_LOG */
 
 /* Increment allocation counter. */
 static void
@@ -253,6 +253,47 @@ log_memstats(int pri)
     }
 }
 
+void
+log_memstats_stderr (const char *prefix)
+{
+  struct mlist *ml;
+  struct memory_list *m;
+  int i;
+  int j = 0;
+
+  for (ml = mlists; ml->list; ml++)
+    {
+      i = 0;
+
+      for (m = ml->list; m->index >= 0; m++)
+        if (m->index && mstat[m->index].alloc)
+          {
+            if (!i)
+              fprintf (stderr,
+                       "%s: memstats: Current memory utilization in module %s:\n",
+                       prefix,
+                       ml->name);
+            fprintf (stderr,
+                     "%s: memstats:  %-30s: %10ld%s\n",
+                     prefix,
+                     m->format,
+                     mstat[m->index].alloc,
+                     mstat[m->index].alloc < 0 ? " (REPORT THIS BUG!)" : "");
+            i = j = 1;
+          }
+    }
+
+  if (j)
+    fprintf (stderr,
+             "%s: memstats: NOTE: If configuration exists, utilization may be "
+             "expected.\n",
+             prefix);
+  else
+    fprintf (stderr,
+             "%s: memstats: No remaining tracked memory utilization.\n",
+             prefix);
+}
+
 static void
 show_separator(struct vty *vty)
 {

lib/memory.h

@@ -81,6 +81,7 @@ extern void mtype_zfree (const char *file, int line, int type,
 extern char *mtype_zstrdup (const char *file, int line, int type,
 		            const char *str);
 extern void memory_init (void);
+extern void log_memstats_stderr (const char *);
 
 /* return number of allocations outstanding for the type */
 extern unsigned long mtype_stats_alloc (int);

lib/routemap.c

@@ -889,6 +889,15 @@ route_map_init (void)
   route_match_vec = vector_init (1);
   route_set_vec = vector_init (1);
 }
+
+void
+route_map_finish (void)
+{
+  vector_free (route_match_vec);
+  route_match_vec = NULL;
+  vector_free (route_set_vec);
+  route_set_vec = NULL;
+}
 
 /* VTY related functions. */
 DEFUN (route_map,

lib/routemap.h

@@ -153,6 +153,7 @@ struct route_map
 /* Prototypes. */
 extern void route_map_init (void);
 extern void route_map_init_vty (void);
+extern void route_map_finish (void);
 
 /* Add match statement to route map. */
 extern int route_map_add_match (struct route_map_index *index,

lib/thread.c

@@ -239,6 +239,15 @@ cpu_record_hash_alloc (struct cpu_thread_history *a)
   return new;
 }
 
+static void
+cpu_record_hash_free (void *a)
+{
+  struct cpu_thread_history *hist = a;
+ 
+  XFREE (MTYPE_THREAD_FUNCNAME, hist->funcname);
+  XFREE (MTYPE_THREAD_STATS, hist);
+}
+
 static inline void 
 vty_out_cpu_thread_history(struct vty* vty,
 			   struct cpu_thread_history *a)
@@ -485,7 +494,8 @@ thread_list_free (struct thread_master *m, struct thread_list *list)
   for (t = list->head; t; t = next)
     {
       next = t->next;
-      XFREE (MTYPE_THREAD_FUNCNAME, t->funcname);
+      if (t->funcname)
+        XFREE (MTYPE_THREAD_FUNCNAME, t->funcname);
       XFREE (MTYPE_THREAD, t);
       list->count--;
       m->alloc--;
@@ -505,6 +515,13 @@ thread_master_free (struct thread_master *m)
   thread_list_free (m, &m->background);
   
   XFREE (MTYPE_THREAD_MASTER, m);
+
+  if (cpu_record)
+    {
+      hash_clean (cpu_record, cpu_record_hash_free);
+      hash_free (cpu_record);
+      cpu_record = NULL;
+    }
 }
 
 /* Thread list is empty or not.  */
@@ -836,6 +853,7 @@ thread_run (struct thread_master *m, struct thread *thread,
 {
   *fetch = *thread;
   thread->type = THREAD_UNUSED;
+  thread->funcname = NULL;  /* thread_call will free fetch's copied pointer */
   thread_add_unuse (m, thread);
   return fetch;
 }
@@ -1079,6 +1097,8 @@ thread_call (struct thread *thread)
 		 realtime/1000, cputime/1000);
     }
 #endif /* CONSUMED_TIME_CHECK */
+
+  XFREE (MTYPE_THREAD_FUNCNAME, thread->funcname);
 }
 
 /* Execute thread */

lib/vty.c

@@ -1034,7 +1034,7 @@ vty_describe_command (struct vty *vty)
 	if (desc->cmd[0] == '\0')
 	  continue;
 	
-	if (strcmp (desc->cmd, "<cr>") == 0)
+	if (strcmp (desc->cmd, command_cr) == 0)
 	  {
 	    desc_cr = desc;
 	    continue;
@@ -2988,3 +2988,17 @@ vty_init (struct thread_master *master_thread)
   install_element (VTY_NODE, &no_vty_ipv6_access_class_cmd);
 #endif /* HAVE_IPV6 */
 }
+
+void
+vty_terminate (void)
+{
+  if (vty_cwd)
+    XFREE (MTYPE_TMP, vty_cwd);
+
+  if (vtyvec && Vvty_serv_thread)
+    {
+      vty_reset ();
+      vector_free (vtyvec);
+      vector_free (Vvty_serv_thread);
+    }
+}

lib/vty.h

@@ -203,6 +203,7 @@ extern char integrate_default[];
 /* Prototypes. */
 extern void vty_init (struct thread_master *);
 extern void vty_init_vtysh (void);
+extern void vty_terminate (void);
 extern void vty_reset (void);
 extern struct vty *vty_new (void);
 extern int vty_out (struct vty *, const char *, ...) PRINTF_ATTRIBUTE(2, 3);

lib/zclient.c

@@ -58,13 +58,11 @@ zclient_new ()
   return zclient;
 }
 
-#if 0
-/* This function is never used.  And it must not be used, because
+/* This function is only called when exiting, because
    many parts of the code do not check for I/O errors, so they could
    reference an invalid pointer if the structure was ever freed.
-*/
 
-/* Free zclient structure. */
+   Free zclient structure. */
 void
 zclient_free (struct zclient *zclient)
 {
@@ -77,7 +75,6 @@ zclient_free (struct zclient *zclient)
 
   XFREE (MTYPE_ZCLIENT, zclient);
 }
-#endif
 
 /* Initialize zebra client.  Argument redist_default is unwanted
    redistribute route type. */

lib/zclient.h

@@ -125,6 +125,7 @@ extern void zclient_init (struct zclient *, int);
 extern int zclient_start (struct zclient *);
 extern void zclient_stop (struct zclient *);
 extern void zclient_reset (struct zclient *);
+extern void zclient_free (struct zclient *);
 
 /* Get TCP socket connection to zebra daemon at loopback address. */
 extern int zclient_socket (void);