diff --git a/lib/ChangeLog b/lib/ChangeLog
index ee20de8..3473257 100644
--- a/lib/ChangeLog
+++ b/lib/ChangeLog
@@ -1,3 +1,25 @@
+2005-04-07 Paul Jakma <paul.jakma@sun.com>
+
+	* linklist.h: Add usage comments.	  
+	  Rename getdata macro to listgetdata.
+	  Rename nextnode to listnextnode and fix its odd behaviour to be
+	  less dangerous.
+	  Make listgetdata macro assert node is not null, NULL list entries
+          should be bug condition.
+          ALL_LIST_ELEMENTS, new macro, forward-referencing macro for use
+          with for loop, Suggested by Jim Carlson of Sun.
+          Add ALL_LIST_ELEMENTS_RO for cases which obviously do not need the
+          "safety" of previous macro.
+	  LISTNODE_ADD and DELETE macros renamed to ATTACH, DETACH, to
+	  distinguish from the similarly named functions, and reflect their
+	  effect better.
+	  Add a QUAGGA_NO_DEPRECATED_INTERFACES define guarded section 
+	  with the old defines which were modified above, 
+	  for backwards compatibility - guarded to prevent Quagga using it..
+	* linklist.c: fix up for linklist.h changes.
+	* *.c: fix up for new list loop macro, try audit other loop
+          usage at same time, to some degree.
+
 2004-04-05 Hasso Tepper <hasso at quagga.net>
 
 	* lib/prefix.[hc]: inet6_ntoa utility function copied from
diff --git a/lib/if.c b/lib/if.c
index dbf4f20..35fe9ca 100644
--- a/lib/if.c
+++ b/lib/if.c
@@ -183,9 +183,8 @@
   struct listnode *node;
   struct interface *ifp;
 
-  for (node = listhead (iflist); node; nextnode (node))
+  for (ALL_LIST_ELEMENTS_RO(iflist, node, ifp))
     {
-      ifp = getdata (node);
       if (ifp->ifindex == index)
 	return ifp;
     }
@@ -216,9 +215,8 @@
   struct listnode *node;
   struct interface *ifp;
 
-  for (node = listhead (iflist); node; nextnode (node))
+  for (ALL_LIST_ELEMENTS_RO (iflist, node, ifp))
     {
-      ifp = getdata (node);
       if (strcmp(name, ifp->name) == 0)
 	return ifp;
     }
@@ -229,15 +227,13 @@
 if_lookup_by_name_len(const char *name, size_t namelen)
 {
   struct listnode *node;
+  struct interface *ifp;
 
   if (namelen > INTERFACE_NAMSIZ)
     return NULL;
 
-  for (node = listhead (iflist); node; nextnode (node))
+  for (ALL_LIST_ELEMENTS_RO (iflist, node, ifp))
     {
-      struct interface *ifp;
-
-      ifp = getdata (node);
       if (!memcmp(name, ifp->name, namelen) && (ifp->name[namelen] == '\0'))
 	return ifp;
     }
@@ -254,14 +250,10 @@
   struct prefix *p;
   struct connected *c;
 
-  for (node = listhead (iflist); node; nextnode (node))
+  for (ALL_LIST_ELEMENTS_RO (iflist, node, ifp))
     {
-      ifp = getdata (node);
-
-      for (cnode = listhead (ifp->connected); cnode; nextnode (cnode))
+      for (ALL_LIST_ELEMENTS_RO (ifp->connected, cnode, c))
 	{
-	  c = getdata (cnode);
-
 	  p = c->address;
 
 	  if (p && p->family == AF_INET)
@@ -293,14 +285,10 @@
 
   match = NULL;
 
-  for (node = listhead (iflist); node; nextnode (node))
+  for (ALL_LIST_ELEMENTS_RO (iflist, node, ifp))
     {
-      ifp = getdata (node);
-
-      for (cnode = listhead (ifp->connected); cnode; nextnode (cnode))
+      for (ALL_LIST_ELEMENTS_RO (ifp->connected, cnode, c))
 	{
-	  c = getdata (cnode);
-
 	  if (c->address && (c->address->family == AF_INET))
 	    {
 	      if (CONNECTED_POINTOPOINT_HOST(c))
@@ -450,6 +438,7 @@
 if_dump (struct interface *ifp)
 {
   struct listnode *node;
+  struct connected *c;
 
   zlog_info ("Interface %s index %d metric %d mtu %d "
 #ifdef HAVE_IPV6
@@ -462,7 +451,7 @@
 #endif /* HAVE_IPV6 */
 	     if_flag_dump (ifp->flags));
   
-  for (node = listhead (ifp->connected); node; nextnode (node))
+  for (ALL_LIST_ELEMENTS_RO (ifp->connected, node, c))
     ;
 }
 
@@ -471,9 +460,10 @@
 if_dump_all ()
 {
   struct listnode *node;
+  void *p;
 
-  for (node = listhead (iflist); node; nextnode (node))
-    if_dump (getdata (node));
+  for (ALL_LIST_ELEMENTS_RO (iflist, node, p))
+    if_dump (p);
 }
 
 DEFUN (interface_desc, 
@@ -581,13 +571,10 @@
   struct connected *ifc;
   struct prefix *p;
 
-  for (node = listhead (iflist); node; nextnode (node))
+  for (ALL_LIST_ELEMENTS_RO (iflist, node, ifp))
     {
-      ifp = getdata (node);
-
-      for (node2 = listhead (ifp->connected); node2; nextnode (node2))
+      for (ALL_LIST_ELEMENTS_RO (ifp->connected, node2, ifc))
 	{
-	  ifc = getdata (node2);
 	  p = ifc->address;
 
 	  if (p->family == AF_INET)
@@ -677,7 +664,7 @@
   /* In case of same prefix come, replace it with new one. */
   for (node = listhead (ifp->connected); node; node = next)
     {
-      ifc = getdata (node);
+      ifc = listgetdata (node);
       next = node->next;
 
       if (connected_same_prefix (ifc->address, p))
@@ -706,10 +693,8 @@
 
   match = NULL;
 
-  for (cnode = listhead (ifp->connected); cnode; nextnode (cnode))
+  for (ALL_LIST_ELEMENTS_RO (ifp->connected, cnode, c))
     {
-      c = getdata (cnode);
-
       if (c->address && (c->address->family == AF_INET))
         {
 	  if (CONNECTED_POINTOPOINT_HOST(c))
@@ -838,7 +823,6 @@
   struct prefix_ipv4 p;
   struct route_node *rn;
   struct interface *ifp;
-  struct listnode *node;
 
   if (addr)
     {
diff --git a/lib/keychain.c b/lib/keychain.c
index a3219ef..2b5b068 100644
--- a/lib/keychain.c
+++ b/lib/keychain.c
@@ -61,13 +61,13 @@
 struct keychain *
 keychain_lookup (const char *name)
 {
-  struct listnode *nn;
+  struct listnode *node;
   struct keychain *keychain;
 
   if (name == NULL)
     return NULL;
 
-  LIST_LOOP (keychain_list, keychain, nn)
+  for (ALL_LIST_ELEMENTS_RO (keychain_list, node, keychain))
     {
       if (strcmp (keychain->name, name) == 0)
 	return keychain;
@@ -127,10 +127,10 @@
 struct key *
 key_lookup (const struct keychain *keychain, u_int32_t index)
 {
-  struct listnode *nn;
+  struct listnode *node;
   struct key *key;
 
-  LIST_LOOP (keychain->key, key, nn)
+  for (ALL_LIST_ELEMENTS_RO (keychain->key, node, key))
     {
       if (key->index == index)
 	return key;
@@ -141,13 +141,13 @@
 struct key *
 key_lookup_for_accept (const struct keychain *keychain, u_int32_t index)
 {
-  struct listnode *nn;
+  struct listnode *node;
   struct key *key;
   time_t now;
 
   now = time (NULL);
 
-  LIST_LOOP (keychain->key, key, nn)
+  for (ALL_LIST_ELEMENTS_RO (keychain->key, node, key))
     {
       if (key->index >= index)
 	{
@@ -165,13 +165,13 @@
 struct key *
 key_match_for_accept (const struct keychain *keychain, const char *auth_str)
 {
-  struct listnode *nn;
+  struct listnode *node;
   struct key *key;
   time_t now;
 
   now = time (NULL);
 
-  LIST_LOOP (keychain->key, key, nn)
+  for (ALL_LIST_ELEMENTS_RO (keychain->key, node, key))
     {
       if (key->accept.start == 0 ||
 	  (key->accept.start <= now &&
@@ -185,13 +185,13 @@
 struct key *
 key_lookup_for_send (const struct keychain *keychain)
 {
-  struct listnode *nn;
+  struct listnode *node;
   struct key *key;
   time_t now;
 
   now = time (NULL);
 
-  LIST_LOOP (keychain->key, key, nn)
+  for (ALL_LIST_ELEMENTS_RO (keychain->key, node, key))
     {
       if (key->send.start == 0)
 	return key;
@@ -881,15 +881,15 @@
 {
   struct keychain *keychain;
   struct key *key;
-  struct listnode *nn;
-  struct listnode *nm;
+  struct listnode *node;
+  struct listnode *knode;
   char buf[BUFSIZ];
 
-  LIST_LOOP (keychain_list, keychain, nn)
+  for (ALL_LIST_ELEMENTS_RO (keychain_list, node, keychain))
     {
       vty_out (vty, "key chain %s%s", keychain->name, VTY_NEWLINE);
       
-      LIST_LOOP (keychain->key, key, nm)
+      for (ALL_LIST_ELEMENTS_RO (keychain->key, knode, key))
 	{
 	  vty_out (vty, " key %d%s", key->index, VTY_NEWLINE);
 
diff --git a/lib/linklist.c b/lib/linklist.c
index 3970c24..4c47153 100644
--- a/lib/linklist.c
+++ b/lib/linklist.c
@@ -247,8 +247,8 @@
   struct listnode *node;
 
   assert(list);
-  for (node = list->head; node; nextnode (node))
-    if (data == getdata (node))
+  for (node = listhead(list); node; node = listnextnode (node))
+    if (data == listgetdata (node))
       return node;
   return NULL;
 }
@@ -317,6 +317,6 @@
 {
   struct listnode *n;
 
-  for (n = listhead (m); n; nextnode (n))
+  for (n = listhead (m); n; n = listnextnode (n))
     listnode_add (l, n->data);
 }
diff --git a/lib/linklist.h b/lib/linklist.h
index b766420..80b21f6 100644
--- a/lib/linklist.h
+++ b/lib/linklist.h
@@ -22,10 +22,15 @@
 #ifndef _ZEBRA_LINKLIST_H
 #define _ZEBRA_LINKLIST_H
 
+/* listnodes must always contain data to be valid. Adding an empty node
+ * to a list is invalid
+ */
 struct listnode 
 {
   struct listnode *next;
   struct listnode *prev;
+  
+  /* private member, use getdata() to retrieve, do not access directly */
   void *data;
 };
 
@@ -33,25 +38,31 @@
 {
   struct listnode *head;
   struct listnode *tail;
+
   /* invariant: count is the number of listnodes in the list */
   unsigned int count;
+
   /*
    * Returns -1 if val1 < val2, 0 if equal?, 1 if val1 > val2.
    * Used as definition of sorted for listnode_add_sort
    */
   int (*cmp) (void *val1, void *val2);
+
+  /* callback to free user-owned data when listnode is deleted. supplying
+   * this callback is very much encouraged!
+   */
   void (*del) (void *val);
 };
 
-#define nextnode(X) ((X) = (X)->next)
+#define listnextnode(X) ((X)->next)
 #define listhead(X) ((X)->head)
 #define listtail(X) ((X)->tail)
 #define listcount(X) ((X)->count)
 #define list_isempty(X) ((X)->head == NULL && (X)->tail == NULL)
-#define getdata(X) ((X)->data)
+#define listgetdata(X) (assert((X)->data != NULL), (X)->data)
 
 /* Prototypes. */
-struct list *list_new();
+struct list *list_new(); /* encouraged: set list.del callback on new lists */
 void list_free (struct list *);
 
 void listnode_add (struct list *, void *);
@@ -72,13 +83,33 @@
 void list_add_node_next (struct list *, struct listnode *, void *);
 void list_add_list (struct list *, struct list *);
 
-/* List iteration macro. */
-#define LIST_LOOP(L,V,N) \
-  for ((N) = (L)->head; (N); (N) = (N)->next) \
-    if (((V) = (N)->data) != NULL)
+/* List iteration macro. 
+ * Usage: for (ALL_LIST_ELEMENTS (...) { ... }
+ * It is safe to delete the listnode using this macro.
+ */
+#define ALL_LIST_ELEMENTS(list,node,nextnode,data) \
+  (node) = listhead(list); \
+  (node) != NULL && \
+    ((data) = listgetdata(node),(nextnode) = listnextnode(node), 1); \
+  (node) = (nextnode)
 
-/* List node add macro.  */
-#define LISTNODE_ADD(L,N) \
+/* read-only list iteration macro.
+ * Usage: as per ALL_LIST_ELEMENTS, but not safe to delete the listnode Only
+ * use this macro when it is *immediately obvious* the listnode is not
+ * deleted in the body of the loop. Does not have forward-reference overhead
+ * of previous macro.
+ */
+#define ALL_LIST_ELEMENTS_RO(list,node,data) \
+  (node) = listhead(list); \
+  (node) != NULL && ((data) = listgetdata(node), 1); \
+  (node) = listnextnode(node)
+
+/* these *do not* cleanup list nodes and referenced data, as the functions
+ * do - these macros simply {de,at}tach a listnode from/to a list.
+ */
+ 
+/* List node attach macro.  */
+#define LISTNODE_ATTACH(L,N) \
   do { \
     (N)->prev = (L)->tail; \
     if ((L)->head == NULL) \
@@ -89,8 +120,8 @@
     (L)->count++; \
   } while (0)
 
-/* List node delete macro.  */
-#define LISTNODE_DELETE(L,N) \
+/* List node detach macro.  */
+#define LISTNODE_DETACH(L,N) \
   do { \
     if ((N)->prev) \
       (N)->prev->next = (N)->next; \
@@ -103,4 +134,15 @@
     (L)->count--; \
   } while (0)
 
+/* Deprecated: 20050406 */
+#if !defined(QUAGGA_NO_DEPRECATED_INTERFACES)
+#warning "Using deprecated libzebra interfaces"
+#define LISTNODE_ADD(L,N) LISTNODE_ATTACH(L,N)
+#define LISTNODE_DELETE(L,N) LISTNODE_DETACH(L,N)
+#define nextnode(X) ((X) = (X)->next)
+#define getdata(X) listgetdata(X)
+#define LIST_LOOP(L,V,N) \
+  for (ALL_LIST_ELEMENTS_RO (L,N,V))
+#endif /* QUAGGA_NO_DEPRECATED_INTERFACES */
+
 #endif /* _ZEBRA_LINKLIST_H */
diff --git a/lib/smux.c b/lib/smux.c
index 0c2dba3..28f402d 100644
--- a/lib/smux.c
+++ b/lib/smux.c
@@ -441,12 +441,11 @@
   int result;
   u_char *statP = NULL;
   WriteMethod *write_method = NULL;
-  struct listnode *node;
+  struct listnode *node, *nnode;
 
   /* Check */
-  for (node = treelist->head; node; node = node->next)
+  for (ALL_LIST_ELEMENTS (treelist, node, nnode, subtree))
     {
-      subtree = node->data;
       subresult = oid_compare_part (reqid, *reqid_len,
                                     subtree->name, subtree->name_len);
 
@@ -509,12 +508,11 @@
   size_t suffix_len;
   int result;
   WriteMethod *write_method=NULL;
-  struct listnode *node;
+  struct listnode *node, *nnode;
 
   /* Check */
-  for (node = treelist->head; node; node = node->next)
+  for (ALL_LIST_ELEMENTS (treelist, node, nnode,subtree))
     {
-      subtree = node->data;
       subresult = oid_compare_part (reqid, *reqid_len, 
 				    subtree->name, subtree->name_len);
 
@@ -578,7 +576,7 @@
   size_t suffix_len;
   int result;
   WriteMethod *write_method=NULL;
-  struct listnode *node;
+  struct listnode *node, *nnode;
 
 
   /* Save incoming request. */
@@ -586,9 +584,8 @@
   savelen = *reqid_len;
 
   /* Check */
-  for (node = treelist->head; node; node = node->next)
+  for (ALL_LIST_ELEMENTS (treelist, node, nnode, subtree))
     {
-      subtree = node->data;
       subresult = oid_compare_part (reqid, *reqid_len, 
 				    subtree->name, subtree->name_len);
 
@@ -1108,17 +1105,15 @@
   long priority;
   long operation;
   struct subtree *subtree;
-  struct listnode *node;
+  struct listnode *node, *nnode;
 
   ret = 0;
 
-  for (node = treelist->head; node; node = node->next)
+  for (ALL_LIST_ELEMENTS (treelist, node, nnode, subtree))
     {
       ptr = buf;
       len = BUFSIZ;
 
-      subtree = node->data;
-
       /* SMUX RReq Header. */
       ptr = asn_build_header (ptr, &len, (u_char) SMUX_RREQ, 0);