[ospfd] Allow ospf_lsa_unlock to NULL out callers' LSA pointers upon free

2006-07-26 Paul Jakma <paul.jakma@sun.com>

	* ospf_lsa.{c,h}: (ospf_lsa_unlock) Change to take a double pointer
	  to the LSA to be 'unlocked', so that, if the LSA is freed, the
	  callers pointer to the LSA can be NULLed out, allowing any further
	  use of that pointer to provoke a crash sooner rather than later.
	* ospf_*.c: (general) Adjust callers of ospf_lsa_unlock to match
	  previous. Try annotate 'locking' somewhat to show which 'locks'
	  are protecting what LSA reference, if not obvious.
	* ospf_opaque.c: (ospf_opaque_lsa_install) Trivial: remove useless
	  goto, replace with return.
	* ospf_packet.c: (ospf_make_ls_ack) Trivial: merge two list loops,
	  the dual-loop predated the delete-safe list-loop macro.

14 files changed, 72 insertions, 83 deletions

diff --git a/ospfd/ChangeLog b/ospfd/ChangeLog
index 2c23774ad..61ca5e0f1 100644
--- a/ospfd/ChangeLog
+++ b/ospfd/ChangeLog
@@ -1,3 +1,17 @@
+2006-07-26 Paul Jakma <paul.jakma@sun.com>
+
+	* ospf_lsa.{c,h}: (ospf_lsa_unlock) Change to take a double pointer
+	  to the LSA to be 'unlocked', so that, if the LSA is freed, the
+	  callers pointer to the LSA can be NULLed out, allowing any further
+	  use of that pointer to provoke a crash sooner rather than later.
+	* ospf_*.c: (general) Adjust callers of ospf_lsa_unlock to match
+	  previous. Try annotate 'locking' somewhat to show which 'locks'
+	  are protecting what LSA reference, if not obvious.
+	* ospf_opaque.c: (ospf_opaque_lsa_install) Trivial: remove useless
+	  goto, replace with return.
+	* ospf_packet.c: (ospf_make_ls_ack) Trivial: merge two list loops,
+	  the dual-loop predated the delete-safe list-loop macro.
+
 2006-07-25 Paul Jakma <paul.jakma@sun.com>
 
 	* ospf_neigbor.h: (struct ospf_neighbor) Add some additional
diff --git a/ospfd/ospf_apiserver.c b/ospfd/ospf_apiserver.c
index 2ee4d3e16..dac4c93fd 100644
--- a/ospfd/ospf_apiserver.c
+++ b/ospfd/ospf_apiserver.c
@@ -1520,7 +1520,7 @@ ospf_apiserver_opaque_lsa_new (struct ospf_area *area,
   if ((new->data = ospf_lsa_data_new (length)) == NULL)
     {
       zlog_warn ("ospf_apiserver_opaque_lsa_new: ospf_lsa_data_new() ?");
-      ospf_lsa_unlock (new);
+      ospf_lsa_unlock (&new);
       stream_free (s);
       return NULL;
     }
@@ -1885,7 +1885,7 @@ ospf_apiserver_lsa_refresher (struct ospf_lsa *lsa)
   if (ospf_lsa_install (ospf, new->oi, new) == NULL)
     {
       zlog_warn ("ospf_apiserver_lsa_refresher: ospf_lsa_install failed");
-      ospf_lsa_unlock (new);
+      ospf_lsa_unlock (&new);
       goto out;
     }
 
diff --git a/ospfd/ospf_ase.c b/ospfd/ospf_ase.c
index f4b285bb0..a4812345c 100644
--- a/ospfd/ospf_ase.c
+++ b/ospfd/ospf_ase.c
@@ -711,7 +711,7 @@ ospf_ase_register_external_lsa (struct ospf_lsa *lsa, struct ospf *top)
   /* We assume that if LSA is deleted from DB
      is is also deleted from this RT */
 
-  listnode_add (lst, ospf_lsa_lock (lsa));
+  listnode_add (lst, ospf_lsa_lock (lsa)); /* external_lsas lst */
 }
 
 void
@@ -730,18 +730,12 @@ ospf_ase_unregister_external_lsa (struct ospf_lsa *lsa, struct ospf *top)
 
   rn = route_node_get (top->external_lsas, (struct prefix *) &p);
   lst = rn->info;
-#ifdef ORIGINAL_CODING
-  assert (lst);
 
-  listnode_delete (lst, lsa);
-  ospf_lsa_unlock (lsa);
-#else /* ORIGINAL_CODING */
   /* XXX lst can be NULL */
   if (lst) {
     listnode_delete (lst, lsa);
-    ospf_lsa_unlock (lsa);
+    ospf_lsa_unlock (&lsa); /* external_lsas list */
   }
-#endif /* ORIGINAL_CODING */
 }
 
 void
@@ -756,7 +750,7 @@ ospf_ase_external_lsas_finish (struct route_table *rt)
     if ((lst = rn->info) != NULL)
       {
 	for (ALL_LIST_ELEMENTS (lst, node, nnode, lsa))
-          ospf_lsa_unlock (lsa);
+          ospf_lsa_unlock (&lsa); /* external_lsas lst */
 	list_delete (lst);
       }
     
diff --git a/ospfd/ospf_flood.c b/ospfd/ospf_flood.c
index d7ab859e9..91cbbf3dd 100644
--- a/ospfd/ospf_flood.c
+++ b/ospfd/ospf_flood.c
@@ -72,7 +72,7 @@ ospf_flood_delayed_lsa_ack (struct ospf_neighbor *inbr, struct ospf_lsa *lsa)
     return;
 
   /* Schedule a delayed LSA Ack to be sent */ 
-  listnode_add (inbr->oi->ls_ack, ospf_lsa_lock (lsa));
+  listnode_add (inbr->oi->ls_ack, ospf_lsa_lock (lsa)); /* delayed LSA Ack */
 }
 
 /* Check LSA is related to external info. */
@@ -134,7 +134,7 @@ ospf_process_self_originated_lsa (struct ospf *ospf,
     case OSPF_ROUTER_LSA:
       /* Originate a new instance and schedule flooding */
       /* It shouldn't be necessary, but anyway */
-      ospf_lsa_unlock (area->router_lsa_self);
+      ospf_lsa_unlock (&area->router_lsa_self);
       area->router_lsa_self = ospf_lsa_lock (new);
 
       ospf_router_lsa_timer_add (area);
@@ -170,7 +170,7 @@ ospf_process_self_originated_lsa (struct ospf *ospf,
               }
 #endif /* HAVE_OPAQUE_LSA */
 
-            ospf_lsa_unlock (oi->network_lsa_self);
+            ospf_lsa_unlock (&oi->network_lsa_self);
             oi->network_lsa_self = ospf_lsa_lock (new);
             
             /* Schedule network-LSA origination. */
@@ -797,7 +797,7 @@ ospf_ls_request_delete (struct ospf_neighbor *nbr, struct ospf_lsa *lsa)
 {
   if (nbr->ls_req_last == lsa)
     {
-      ospf_lsa_unlock (nbr->ls_req_last);
+      ospf_lsa_unlock (&nbr->ls_req_last);
       nbr->ls_req_last = NULL;
     }
 
@@ -813,7 +813,7 @@ ospf_ls_request_delete (struct ospf_neighbor *nbr, struct ospf_lsa *lsa)
 void
 ospf_ls_request_delete_all (struct ospf_neighbor *nbr)
 {
-  ospf_lsa_unlock (nbr->ls_req_last);
+  ospf_lsa_unlock (&nbr->ls_req_last);
   nbr->ls_req_last = NULL;
   ospf_lsdb_delete_all (&nbr->ls_req);
 }
@@ -922,7 +922,7 @@ ospf_ls_retransmit_clear (struct ospf_neighbor *nbr)
 	  ospf_ls_retransmit_delete (nbr, lsa);
     }
 
-  ospf_lsa_unlock (nbr->ls_req_last);
+  ospf_lsa_unlock (&nbr->ls_req_last);
   nbr->ls_req_last = NULL;
 }
 
diff --git a/ospfd/ospf_interface.c b/ospfd/ospf_interface.c
index 2c2c07497..31275f89d 100644
--- a/ospfd/ospf_interface.c
+++ b/ospfd/ospf_interface.c
@@ -289,7 +289,7 @@ ospf_if_cleanup (struct ospf_interface *oi)
 
   /* Cleanup Link State Acknowlegdment list. */
   for (ALL_LIST_ELEMENTS (oi->ls_ack, node, nnode, lsa))
-    ospf_lsa_unlock (lsa);
+    ospf_lsa_unlock (&lsa); /* oi->ls_ack */
   list_delete_all_node (oi->ls_ack);
 
   oi->crypt_seqnum = 0;
@@ -302,7 +302,7 @@ ospf_if_cleanup (struct ospf_interface *oi)
   oi->nbr_self = ospf_nbr_new (oi);
   ospf_nbr_add_self (oi);
   
-  ospf_lsa_unlock (oi->network_lsa_self);
+  ospf_lsa_unlock (&oi->network_lsa_self);
   oi->network_lsa_self = NULL;
   OSPF_TIMER_OFF (oi->t_network_lsa_self);
 }
diff --git a/ospfd/ospf_ism.c b/ospfd/ospf_ism.c
index 0875e92dd..829ea00a1 100644
--- a/ospfd/ospf_ism.c
+++ b/ospfd/ospf_ism.c
@@ -593,7 +593,7 @@ ism_change_state (struct ospf_interface *oi, int state)
 	  OSPF_TIMER_OFF (oi->t_network_lsa_self);
 	}
 
-      ospf_lsa_unlock (oi->network_lsa_self);
+      ospf_lsa_unlock (&oi->network_lsa_self);
       oi->network_lsa_self = NULL;
     }
 
diff --git a/ospfd/ospf_lsa.c b/ospfd/ospf_lsa.c
index 8b5c6eb81..b99b931c1 100644
--- a/ospfd/ospf_lsa.c
+++ b/ospfd/ospf_lsa.c
@@ -294,20 +294,21 @@ ospf_lsa_lock (struct ospf_lsa *lsa)
 
 /* Unlock LSA. */
 void
-ospf_lsa_unlock (struct ospf_lsa *lsa)
+ospf_lsa_unlock (struct ospf_lsa **lsa)
 {
   /* This is sanity check. */
-  if (!lsa)
+  if (!lsa || !*lsa)
     return;
   
-  lsa->lock--;
+  (*lsa)->lock--;
 
-  assert (lsa->lock >= 0);
+  assert ((*lsa)->lock >= 0);
 
-  if (lsa->lock == 0)
+  if ((*lsa)->lock == 0)
     {
-      assert (CHECK_FLAG (lsa->flags, OSPF_LSA_DISCARD));
-      ospf_lsa_free (lsa);
+      assert (CHECK_FLAG ((*lsa)->flags, OSPF_LSA_DISCARD));
+      ospf_lsa_free (*lsa);
+      *lsa = NULL;
     }
 }
 
@@ -318,7 +319,7 @@ ospf_lsa_discard (struct ospf_lsa *lsa)
   if (!CHECK_FLAG (lsa->flags, OSPF_LSA_DISCARD))
     {
       SET_FLAG (lsa->flags, OSPF_LSA_DISCARD);
-      ospf_lsa_unlock (lsa);
+      ospf_lsa_unlock (&lsa);
     }
 }
 
@@ -1044,7 +1045,7 @@ ospf_router_lsa_update_timer (struct thread *thread)
 	    zlog_debug("LSA[Type%d:%s]: Refresh router-LSA for Area %s",
 		      lsa->data->type, inet_ntoa (lsa->data->id), area_str);
 	  ospf_lsa_flush_area (lsa, area);
-	  ospf_lsa_unlock (area->router_lsa_self);
+	  ospf_lsa_unlock (&area->router_lsa_self);
 	  area->router_lsa_self = NULL;
 
 	  /* Refresh router-LSA, (not install) and flood through area. */
@@ -1850,7 +1851,7 @@ ospf_install_flood_nssa (struct ospf *ospf,
 	  {
 	    if (IS_DEBUG_OSPF_NSSA)
 	      zlog_debug ("LSA[Type-7]: Could not build FWD-ADDR");
-	    ospf_lsa_discard(new);
+	    ospf_lsa_discard (new);
 	    return;
 	  }
 	}
@@ -2518,7 +2519,7 @@ ospf_router_lsa_install (struct ospf *ospf,
                           ospf_router_lsa_timer, OSPF_LS_REFRESH_TIME);
       
       /* Set self-originated router-LSA. */
-      ospf_lsa_unlock (area->router_lsa_self);
+      ospf_lsa_unlock (&area->router_lsa_self);
       area->router_lsa_self = ospf_lsa_lock (new);
 
       if (IS_DEBUG_OSPF (lsa, LSA_INSTALL))
@@ -2562,7 +2563,7 @@ ospf_network_lsa_install (struct ospf *ospf,
 			       ospf_network_lsa_refresh_timer,
 			       OSPF_LS_REFRESH_TIME);
 
-      ospf_lsa_unlock (oi->network_lsa_self);
+      ospf_lsa_unlock (&oi->network_lsa_self);
       oi->network_lsa_self = ospf_lsa_lock (new);
     }
 
@@ -3067,7 +3068,7 @@ ospf_lsa_maxage_delete (struct ospf *ospf, struct ospf_lsa *lsa)
   if ((n = listnode_lookup (ospf->maxage_lsa, lsa)))
     {
       list_delete_node (ospf->maxage_lsa, n);
-      ospf_lsa_unlock (lsa);
+      ospf_lsa_unlock (&lsa); /* maxage_lsa */
     }
 }
 
@@ -3482,7 +3483,7 @@ ospf_flush_self_originated_lsas_now (struct ospf *ospf)
             zlog_debug ("LSA[Type%d:%s]: Schedule self-originated LSA to FLUSH", lsa->data->type, inet_ntoa (lsa->data->id));
 
           ospf_lsa_flush_area (lsa, area);
-          ospf_lsa_unlock (area->router_lsa_self);
+          ospf_lsa_unlock (&area->router_lsa_self);
           area->router_lsa_self = NULL;
           OSPF_TIMER_OFF (area->t_router_lsa_self);
         }
@@ -3497,7 +3498,7 @@ ospf_flush_self_originated_lsas_now (struct ospf *ospf)
                 zlog_debug ("LSA[Type%d:%s]: Schedule self-originated LSA to FLUSH", lsa->data->type, inet_ntoa (lsa->data->id));
 
               ospf_lsa_flush_area (oi->network_lsa_self, area);
-              ospf_lsa_unlock (oi->network_lsa_self);
+              ospf_lsa_unlock (&oi->network_lsa_self);
               oi->network_lsa_self = NULL;
               OSPF_TIMER_OFF (oi->t_network_lsa_self);
             }
@@ -3666,7 +3667,7 @@ ospf_lsa_action (struct thread *t)
       break;
     }
 
-  ospf_lsa_unlock (data->lsa);
+  ospf_lsa_unlock (&data->lsa); /* Message */
   XFREE (MTYPE_OSPF_MESSAGE, data);
   return 0;
 }
@@ -3681,7 +3682,7 @@ ospf_schedule_lsa_flood_area (struct ospf_area *area, struct ospf_lsa *lsa)
 
   data->action = LSA_ACTION_FLOOD_AREA;
   data->area = area;
-  data->lsa  = ospf_lsa_lock (lsa);
+  data->lsa  = ospf_lsa_lock (lsa); /* Message / Flood area */
 
   thread_add_event (master, ospf_lsa_action, data, 0);
 }
@@ -3696,7 +3697,7 @@ ospf_schedule_lsa_flush_area (struct ospf_area *area, struct ospf_lsa *lsa)
 
   data->action = LSA_ACTION_FLUSH_AREA;
   data->area = area;
-  data->lsa  = ospf_lsa_lock (lsa);
+  data->lsa  = ospf_lsa_lock (lsa); /* Message / Flush area */
 
   thread_add_event (master, ospf_lsa_action, data, 0);
 }
@@ -3779,7 +3780,8 @@ ospf_refresher_register_lsa (struct ospf *ospf, struct ospf_lsa *lsa)
 		   inet_ntoa (lsa->data->id), LS_AGE (lsa), index);
       if (!ospf->lsa_refresh_queue.qs[index])
 	ospf->lsa_refresh_queue.qs[index] = list_new ();
-      listnode_add (ospf->lsa_refresh_queue.qs[index], ospf_lsa_lock (lsa));
+      listnode_add (ospf->lsa_refresh_queue.qs[index],
+                    ospf_lsa_lock (lsa)); /* lsa_refresh_queue */
       lsa->refresh_list = index;
       if (IS_DEBUG_OSPF (lsa, LSA_REFRESH))
         zlog_debug ("LSA[Refresh:%s]: ospf_refresher_register_lsa(): "
@@ -3801,7 +3803,7 @@ ospf_refresher_unregister_lsa (struct ospf *ospf, struct ospf_lsa *lsa)
 	  list_free (refresh_list);
 	  ospf->lsa_refresh_queue.qs[lsa->refresh_list] = NULL;
 	}
-      ospf_lsa_unlock (lsa);
+      ospf_lsa_unlock (&lsa); /* lsa_refresh_queue */
       lsa->refresh_list = -1;
     }
 }
@@ -3855,7 +3857,7 @@ ospf_lsa_refresh_walker (struct thread *t)
 		           inet_ntoa (lsa->data->id), lsa, i);
 	      
 	      list_delete_node (refresh_list, node);
-	      ospf_lsa_unlock (lsa);
+	      ospf_lsa_unlock (&lsa); /* lsa_refresh_queue */
 	      lsa->refresh_list = -1;
 	      listnode_add (lsa_to_refresh, lsa);
 	    }
diff --git a/ospfd/ospf_lsa.h b/ospfd/ospf_lsa.h
index 9e480dee5..8dd054c31 100644
--- a/ospfd/ospf_lsa.h
+++ b/ospfd/ospf_lsa.h
@@ -245,7 +245,7 @@ extern struct ospf_lsa *ospf_lsa_new (void);
 extern struct ospf_lsa *ospf_lsa_dup (struct ospf_lsa *);
 extern void ospf_lsa_free (struct ospf_lsa *);
 extern struct ospf_lsa *ospf_lsa_lock (struct ospf_lsa *);
-extern void ospf_lsa_unlock (struct ospf_lsa *);
+extern void ospf_lsa_unlock (struct ospf_lsa **);
 extern void ospf_lsa_discard (struct ospf_lsa *);
 
 extern struct lsa_header *ospf_lsa_data_new (size_t);
diff --git a/ospfd/ospf_lsdb.c b/ospfd/ospf_lsdb.c
index b161b8067..28d92bde2 100644
--- a/ospfd/ospf_lsdb.c
+++ b/ospfd/ospf_lsdb.c
@@ -108,7 +108,7 @@ ospf_lsdb_add (struct ospf_lsdb *lsdb, struct ospf_lsa *lsa)
       old = rn->info;
       lsdb->type[old->data->type].checksum -= ntohs(old->data->checksum);
 
-      ospf_lsa_unlock (rn->info);
+      ospf_lsa_unlock (&rn->info);
       route_unlock_node (rn);
     }
 
@@ -161,7 +161,7 @@ ospf_lsdb_delete (struct ospf_lsdb *lsdb, struct ospf_lsa *lsa)
         if (lsdb->del_lsa_hook != NULL)
           (* lsdb->del_lsa_hook)(lsa);
 #endif /* MONITOR_LSDB_CHANGE */
-	ospf_lsa_unlock (lsa);
+	ospf_lsa_unlock (&lsa);
 	return;
       }
 }
@@ -191,7 +191,7 @@ ospf_lsdb_delete_all (struct ospf_lsdb *lsdb)
             if (lsdb->del_lsa_hook != NULL)
               (* lsdb->del_lsa_hook)(lsa);
 #endif /* MONITOR_LSDB_CHANGE */
-	    ospf_lsa_unlock (lsa);
+	    ospf_lsa_unlock (&lsa);
 	  }
     }
 }
diff --git a/ospfd/ospf_nsm.c b/ospfd/ospf_nsm.c
index 8329a4f36..e3517cdd2 100644
--- a/ospfd/ospf_nsm.c
+++ b/ospfd/ospf_nsm.c
@@ -723,7 +723,7 @@ nsm_change_state (struct ospf_neighbor *nbr, int state)
 	  if (oi->network_lsa_self && oi->full_nbrs == 0)
 	    {
 	      ospf_lsa_flush_area (oi->network_lsa_self, oi->area);
-	      ospf_lsa_unlock (oi->network_lsa_self);
+	      ospf_lsa_unlock (&oi->network_lsa_self);
 	      oi->network_lsa_self = NULL;
 	      OSPF_TIMER_OFF (oi->t_network_lsa_self);
 	    }
diff --git a/ospfd/ospf_opaque.c b/ospfd/ospf_opaque.c
index f2496cfd7..0b6ac4cb7 100644
--- a/ospfd/ospf_opaque.c
+++ b/ospfd/ospf_opaque.c
@@ -708,7 +708,7 @@ free_opaque_info_per_id (void *val)
 
   OSPF_TIMER_OFF (oipi->t_opaque_lsa_self);
   if (oipi->lsa != NULL)
-    ospf_lsa_unlock (oipi->lsa);
+    ospf_lsa_unlock (&oipi->lsa);
   XFREE (MTYPE_OPAQUE_INFO_PER_ID, oipi);
   return;
 }
@@ -1554,7 +1554,7 @@ ospf_opaque_lsa_install (struct ospf_lsa *lsa, int rt_recalc)
   if ((oipt = lookup_opaque_info_by_type (lsa)) != NULL
       && (oipi = lookup_opaque_info_by_id (oipt, lsa)) != NULL)
     {
-      ospf_lsa_unlock (oipi->lsa);
+      ospf_lsa_unlock (&oipi->lsa);
       oipi->lsa = ospf_lsa_lock (lsa);
     }
   /* Register the new lsa entry and get its control info. */
@@ -2234,7 +2234,7 @@ ospf_opaque_self_originated_lsa_received (struct ospf_neighbor *nbr,
   u_char before;
 
   if ((top = oi_to_top (nbr->oi)) == NULL)
-    goto out;
+    return;
 
   before = IS_OPAQUE_LSA_ORIGINATION_BLOCKED (top->opaque);
 
@@ -2259,7 +2259,7 @@ ospf_opaque_self_originated_lsa_received (struct ospf_neighbor *nbr,
       break;
     default:
       zlog_warn ("ospf_opaque_self_originated_lsa_received: Unexpected LSA-type(%u)", lsa->data->type);
-      goto out;
+      return;
     }
 
   ospf_lsa_discard (lsa); /* List "lsas" will be deleted by caller. */
@@ -2269,9 +2269,6 @@ ospf_opaque_self_originated_lsa_received (struct ospf_neighbor *nbr,
       if (IS_DEBUG_OSPF_EVENT)
         zlog_debug ("Block Opaque-LSA origination: OFF -> ON");
     }
-
-out:
-  return;
 }
 
 void
diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
index 788daba3a..44dca1817 100644
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -2764,7 +2764,7 @@ ospf_make_ls_req_func (struct stream *s, u_int16_t *length,
   stream_put_ipv4 (s, lsa->data->id.s_addr);
   stream_put_ipv4 (s, lsa->data->adv_router.s_addr);
   
-  ospf_lsa_unlock (nbr->ls_req_last);
+  ospf_lsa_unlock (&nbr->ls_req_last);
   nbr->ls_req_last = ospf_lsa_lock (lsa);
   
   *length += 12;
@@ -2860,7 +2860,7 @@ ospf_make_ls_upd (struct ospf_interface *oi, struct list *update, struct stream
       count++;
 
       list_delete_node (update, node);
-      ospf_lsa_unlock (lsa);
+      ospf_lsa_unlock (&lsa); /* oi->ls_upd_queue */
     }
 
   /* Now set #LSAs. */
@@ -2874,17 +2874,13 @@ ospf_make_ls_upd (struct ospf_interface *oi, struct list *update, struct stream
 static int
 ospf_make_ls_ack (struct ospf_interface *oi, struct list *ack, struct stream *s)
 {
-  struct list *rm_list;
-  struct listnode *node;
+  struct listnode *node, *nnode;
   u_int16_t length = OSPF_LS_ACK_MIN_SIZE;
   unsigned long delta = stream_get_endp(s) + 24;
   struct ospf_lsa *lsa;
 
-  rm_list = list_new ();
-  
-  for (ALL_LIST_ELEMENTS_RO (ack, node, lsa))
+  for (ALL_LIST_ELEMENTS (ack, node, nnode, lsa))
     {
-      lsa = listgetdata (node);
       assert (lsa);
       
       if (length + delta > ospf_packet_max (oi))
@@ -2893,21 +2889,10 @@ ospf_make_ls_ack (struct ospf_interface *oi, struct list *ack, struct stream *s)
       stream_put (s, lsa->data, OSPF_LSA_HEADER_SIZE);
       length += OSPF_LSA_HEADER_SIZE;
       
-      listnode_add (rm_list, lsa);
-    }
-  
-  /* Remove LSA from LS-Ack list. */
-  /* XXX: this loop should be removed and the list move done in previous
-   * loop
-   */
-  for (ALL_LIST_ELEMENTS_RO (rm_list, node, lsa))
-    {
       listnode_delete (ack, lsa);
-      ospf_lsa_unlock (lsa);
+      ospf_lsa_unlock (&lsa); /* oi->ls_ack_direct.ls_ack */
     }
   
-  list_delete (rm_list);
-  
   return length;
 }
 
@@ -3396,10 +3381,7 @@ ospf_ls_upd_send (struct ospf_neighbor *nbr, struct list *update, int flag)
     rn->info = list_new ();
 
   for (ALL_LIST_ELEMENTS_RO (update, node, lsa))
-    {
-      ospf_lsa_lock (lsa);
-      listnode_add (rn->info, lsa);
-    }
+    listnode_add (rn->info, ospf_lsa_lock (lsa)); /* oi->ls_upd_queue */
 
   if (oi->t_ls_upd_event == NULL)
     oi->t_ls_upd_event =
diff --git a/ospfd/ospf_te.c b/ospfd/ospf_te.c
index 10a94b8d8..a3ebe62e3 100644
--- a/ospfd/ospf_te.c
+++ b/ospfd/ospf_te.c
@@ -904,7 +904,7 @@ ospf_mpls_te_lsa_new (struct ospf_area *area, struct mpls_te_link *lp)
   if ((new->data = ospf_lsa_data_new (length)) == NULL)
     {
       zlog_warn ("ospf_mpls_te_lsa_new: ospf_lsa_data_new() ?");
-      ospf_lsa_unlock (new);
+      ospf_lsa_unlock (&new);
       new = NULL;
       stream_free (s);
       goto out;
@@ -936,7 +936,7 @@ ospf_mpls_te_lsa_originate1 (struct ospf_area *area, struct mpls_te_link *lp)
   if (ospf_lsa_install (area->ospf, NULL/*oi*/, new) == NULL)
     {
       zlog_warn ("ospf_mpls_te_lsa_originate1: ospf_lsa_install() ?");
-      ospf_lsa_unlock (new);
+      ospf_lsa_unlock (&new);
       goto out;
     }
 
@@ -1054,7 +1054,7 @@ ospf_mpls_te_lsa_refresh (struct ospf_lsa *lsa)
   if (ospf_lsa_install (area->ospf, NULL/*oi*/, new) == NULL)
     {
       zlog_warn ("ospf_mpls_te_lsa_refresh: ospf_lsa_install() ?");
-      ospf_lsa_unlock (new);
+      ospf_lsa_unlock (&new);
       goto out;
     }
 
diff --git a/ospfd/ospfd.c b/ospfd/ospfd.c
index 79c4543f4..ef8272b04 100644
--- a/ospfd/ospfd.c
+++ b/ospfd/ospfd.c
@@ -475,7 +475,7 @@ ospf_finish_final (struct ospf *ospf)
   ospf_lsdb_free (ospf->lsdb);
 
   for (ALL_LIST_ELEMENTS (ospf->maxage_lsa, node, nnode, lsa))
-    ospf_lsa_unlock (lsa);
+    ospf_lsa_unlock (&lsa); /* maxage_lsa */
 
   list_delete (ospf->maxage_lsa);
 
@@ -592,7 +592,7 @@ ospf_area_free (struct ospf_area *area)
   ospf_lsdb_delete_all (area->lsdb);
   ospf_lsdb_free (area->lsdb);
 
-  ospf_lsa_unlock (area->router_lsa_self);
+  ospf_lsa_unlock (&area->router_lsa_self);
   
   route_table_finish (area->ranges);
   list_delete (area->oiflist);
@@ -905,7 +905,7 @@ ospf_ls_upd_queue_empty (struct ospf_interface *oi)
     if ((lst = (struct list *) rn->info))
       {
 	for (ALL_LIST_ELEMENTS (lst, node, nnode, lsa))
-          ospf_lsa_unlock (lsa);
+          ospf_lsa_unlock (&lsa); /* oi->ls_upd_queue */
 	list_free (lst);
 	rn->info = NULL;
       }