ospf6d: Convert ospf6_lsa_unlock to a better api

Make the ospf6_lsa_unlock take the same parameters that the ospf_lsa_unlock does to make it consistent and to also ensure that no-one can make the mistake of getting the pointer cleared up. Signed-off-by: Donald Sharp <sharpd@nvidia.com>
louis-6wind · Jul 12, 2023 · 0cbd585 · 0cbd585
1 parent c362e27
commit 0cbd585
Show file tree

Hide file tree

Showing 10 changed files with 37 additions and 36 deletions.
diff --git a/ospf6d/ospf6_flood.c b/ospf6d/ospf6_flood.c
@@ -389,7 +389,7 @@ void ospf6_flood_interface(struct ospf6_neighbor *from, struct ospf6_lsa *lsa,
 					if (req == on->last_ls_req) {
 						/* sanity check refcount */
 						assert(req->lock >= 2);
-						req = ospf6_lsa_unlock(req);
+						ospf6_lsa_unlock(&req);
 						on->last_ls_req = NULL;
 					}
 					if (req)
@@ -406,7 +406,7 @@ void ospf6_flood_interface(struct ospf6_neighbor *from, struct ospf6_lsa *lsa,
 						zlog_debug(
 							"Received is newer, remove requesting");
 					if (req == on->last_ls_req) {
-						req = ospf6_lsa_unlock(req);
+						ospf6_lsa_unlock(&req);
 						on->last_ls_req = NULL;
 					}
 					if (req)

diff --git a/ospf6d/ospf6_gr.c b/ospf6d/ospf6_gr.c
@@ -294,7 +294,7 @@ static int ospf6_router_lsa_contains_adj(struct ospf6_area *area,
 				continue;
 
 			if (lsdesc->neighbor_router_id == neighbor_router_id) {
-				ospf6_lsa_unlock(lsa);
+				ospf6_lsa_unlock(&lsa);
 				return RTR_LSA_ADJ_FOUND;
 			}
 		}
@@ -514,7 +514,7 @@ static bool ospf6_gr_check_adjs(struct ospf6 *ospf6)
 					   lsa_self)) {
 			found = true;
 			if (!ospf6_gr_check_adjs_lsa(area, lsa_self)) {
-				ospf6_lsa_unlock(lsa_self);
+				ospf6_lsa_unlock(&lsa_self);
 				return false;
 			}
 		}

diff --git a/ospf6d/ospf6_gr_helper.c b/ospf6d/ospf6_gr_helper.c
@@ -228,9 +228,9 @@ static bool ospf6_check_chg_in_rxmt_list(struct ospf6_neighbor *nbr)
 					  lsa->header->adv_router, lsa->lsdb);
 
 		if (lsa_in_db && lsa_in_db->tobe_acknowledged) {
-			ospf6_lsa_unlock(lsa);
+			ospf6_lsa_unlock(&lsa);
 			if (lsanext)
-				ospf6_lsa_unlock(lsanext);
+				ospf6_lsa_unlock(&lsanext);
 
 			return OSPF6_TRUE;
 		}

diff --git a/ospf6d/ospf6_lsa.c b/ospf6d/ospf6_lsa.c
@@ -797,17 +797,17 @@ struct ospf6_lsa *ospf6_lsa_lock(struct ospf6_lsa *lsa)
 }
 
 /* decrement reference counter of struct ospf6_lsa */
-struct ospf6_lsa *ospf6_lsa_unlock(struct ospf6_lsa *lsa)
+void ospf6_lsa_unlock(struct ospf6_lsa **lsa)
 {
 	/* decrement reference counter */
-	assert(lsa->lock > 0);
-	lsa->lock--;
+	assert((*lsa)->lock > 0);
+	(*lsa)->lock--;
 
-	if (lsa->lock != 0)
-		return lsa;
+	if ((*lsa)->lock != 0)
+		return;
 
-	ospf6_lsa_delete(lsa);
-	return NULL;
+	ospf6_lsa_delete(*lsa);
+	*lsa = NULL;
 }
 
 

diff --git a/ospf6d/ospf6_lsa.h b/ospf6d/ospf6_lsa.h
@@ -239,7 +239,7 @@ extern void ospf6_lsa_delete(struct ospf6_lsa *lsa);
 extern struct ospf6_lsa *ospf6_lsa_copy(struct ospf6_lsa *lsa);
 
 extern struct ospf6_lsa *ospf6_lsa_lock(struct ospf6_lsa *lsa);
-extern struct ospf6_lsa *ospf6_lsa_unlock(struct ospf6_lsa *lsa);
+extern void ospf6_lsa_unlock(struct ospf6_lsa **lsa);
 
 extern void ospf6_lsa_expire(struct event *thread);
 extern void ospf6_lsa_refresh(struct event *thread);

diff --git a/ospf6d/ospf6_lsdb.c b/ospf6d/ospf6_lsdb.c
@@ -139,7 +139,7 @@ void ospf6_lsdb_add(struct ospf6_lsa *lsa, struct ospf6_lsdb *lsdb)
 		}
 		/* to free the lookup lock in node get*/
 		route_unlock_node(current);
-		ospf6_lsa_unlock(old);
+		ospf6_lsa_unlock(&old);
 	}
 
 	ospf6_lsdb_count_assert(lsdb);
@@ -168,7 +168,7 @@ void ospf6_lsdb_remove(struct ospf6_lsa *lsa, struct ospf6_lsdb *lsdb)
 
 	route_unlock_node(node); /* to free the lookup lock */
 	route_unlock_node(node); /* to free the original lock */
-	ospf6_lsa_unlock(lsa);
+	ospf6_lsa_unlock(&lsa);
 
 	ospf6_lsdb_count_assert(lsdb);
 }
@@ -237,7 +237,7 @@ struct ospf6_lsa *ospf6_find_inter_prefix_lsa(struct ospf6 *ospf6,
 		ospf6_prefix_in6_addr(&prefix.u.prefix6, prefix_lsa,
 				      &prefix_lsa->prefix);
 		if (prefix_same(p, &prefix)) {
-			ospf6_lsa_unlock(lsa);
+			ospf6_lsa_unlock(&lsa);
 			return lsa;
 		}
 	}
@@ -328,7 +328,7 @@ struct ospf6_lsa *ospf6_lsdb_next(const struct route_node *iterend,
 {
 	struct route_node *node = lsa->rn;
 
-	ospf6_lsa_unlock(lsa);
+	ospf6_lsa_unlock(&lsa);
 
 	do
 		node = route_next_until(node, iterend);
@@ -361,7 +361,7 @@ void ospf6_lsdb_lsa_unlock(struct ospf6_lsa *lsa)
 	if (lsa != NULL) {
 		if (lsa->rn != NULL)
 			route_unlock_node(lsa->rn);
-		ospf6_lsa_unlock(lsa);
+		ospf6_lsa_unlock(&lsa);
 	}
 }
 

diff --git a/ospf6d/ospf6_lsdb.h b/ospf6d/ospf6_lsdb.h
@@ -63,11 +63,11 @@ extern struct ospf6_lsa *ospf6_lsdb_next(const struct route_node *iterend,
  * it really early.
  */
 #define ALL_LSDB(lsdb, lsa, lsanext)                                           \
-	const struct route_node *iterend =                                     \
-		ospf6_lsdb_head(lsdb, 0, 0, 0, &lsa);                          \
-	(lsa) != NULL && ospf6_lsa_lock(lsa)                                   \
-		&& ((lsanext) = ospf6_lsdb_next(iterend, (lsa)), 1);           \
-	ospf6_lsa_unlock(lsa), (lsa) = (lsanext)
+	const struct route_node *iterend = ospf6_lsdb_head(lsdb, 0, 0, 0,      \
+							   &lsa);              \
+	(lsa) != NULL && ospf6_lsa_lock(lsa) &&                                \
+		((lsanext) = ospf6_lsdb_next(iterend, (lsa)), 1);              \
+	ospf6_lsa_unlock(&lsa), (lsa) = (lsanext)
 
 extern void ospf6_lsdb_remove_all(struct ospf6_lsdb *lsdb);
 extern void ospf6_lsdb_lsa_unlock(struct ospf6_lsa *lsa);

diff --git a/ospf6d/ospf6_message.c b/ospf6d/ospf6_message.c
@@ -2335,9 +2335,9 @@ static uint16_t ospf6_make_dbdesc(struct ospf6_neighbor *on, struct stream *s)
 			if ((length + sizeof(struct ospf6_lsa_header)
 			     + OSPF6_HEADER_SIZE)
 			    > ospf6_packet_max(on->ospf6_if)) {
-				ospf6_lsa_unlock(lsa);
+				ospf6_lsa_unlock(&lsa);
 				if (lsanext)
-					ospf6_lsa_unlock(lsanext);
+					ospf6_lsa_unlock(&lsanext);
 				break;
 			}
 			stream_put(s, lsa->header,
@@ -2415,9 +2415,9 @@ void ospf6_dbdesc_send_newone(struct event *thread)
 
 		if (size + sizeof(struct ospf6_lsa_header)
 		    > ospf6_packet_max(on->ospf6_if)) {
-			ospf6_lsa_unlock(lsa);
+			ospf6_lsa_unlock(&lsa);
 			if (lsanext)
-				ospf6_lsa_unlock(lsanext);
+				ospf6_lsa_unlock(&lsanext);
 			break;
 		}
 
@@ -2447,9 +2447,9 @@ static uint16_t ospf6_make_lsreq(struct ospf6_neighbor *on, struct stream *s)
 	for (ALL_LSDB(on->request_list, lsa, lsanext)) {
 		if ((length + OSPF6_HEADER_SIZE)
 		    > ospf6_packet_max(on->ospf6_if)) {
-			ospf6_lsa_unlock(lsa);
+			ospf6_lsa_unlock(&lsa);
 			if (lsanext)
-				ospf6_lsa_unlock(lsanext);
+				ospf6_lsa_unlock(&lsanext);
 			break;
 		}
 		stream_putw(s, 0); /* reserved */
@@ -2462,7 +2462,7 @@ static uint16_t ospf6_make_lsreq(struct ospf6_neighbor *on, struct stream *s)
 
 	if (last_req != NULL) {
 		if (on->last_ls_req != NULL)
-			on->last_ls_req = ospf6_lsa_unlock(on->last_ls_req);
+			ospf6_lsa_unlock(&on->last_ls_req);
 
 		ospf6_lsa_lock(last_req);
 		on->last_ls_req = last_req;
@@ -2944,9 +2944,9 @@ static uint16_t ospf6_make_lsack_interface(struct ospf6_interface *oi,
 			event_add_event(master, ospf6_lsack_send_interface, oi,
 					0, &oi->thread_send_lsack);
 
-			ospf6_lsa_unlock(lsa);
+			ospf6_lsa_unlock(&lsa);
 			if (lsanext)
-				ospf6_lsa_unlock(lsanext);
+				ospf6_lsa_unlock(&lsanext);
 			break;
 		}
 		ospf6_lsa_age_update_to_send(lsa, oi->transdelay);

diff --git a/ospf6d/ospf6_neighbor.c b/ospf6d/ospf6_neighbor.c
@@ -98,9 +98,10 @@ static void ospf6_neighbor_clear_ls_lists(struct ospf6_neighbor *on)
 
 	ospf6_lsdb_remove_all(on->summary_list);
 	if (on->last_ls_req) {
-		ospf6_lsa_unlock(on->last_ls_req);
+		ospf6_lsa_unlock(&on->last_ls_req);
 		on->last_ls_req = NULL;
 	}
+
 	ospf6_lsdb_remove_all(on->request_list);
 	for (ALL_LSDB(on->retrans_list, lsa, lsanext)) {
 		ospf6_decrement_retrans_count(lsa);

diff --git a/tests/ospf6d/test_lsdb.c b/tests/ospf6d/test_lsdb.c
@@ -59,7 +59,7 @@ DEFPY(lsa_set, lsa_set_cmd,
 
 	lsa_check_resize(idx + 1);
 	if (lsas[idx])
-		ospf6_lsa_unlock(lsas[idx]);
+		ospf6_lsa_unlock(&lsas[idx]);
 	lsas[idx] = ospf6_lsa_create_headeronly(&hdr);
 	ospf6_lsa_lock(lsas[idx]);
 	return CMD_SUCCESS;
@@ -75,7 +75,7 @@ DEFPY(lsa_drop, lsa_drop_cmd,
 		return CMD_SUCCESS;
 	if (lsas[idx]->lock != 1)
 		vty_out(vty, "refcount at %u\n", lsas[idx]->lock);
-	ospf6_lsa_unlock(lsas[idx]);
+	ospf6_lsa_unlock(&lsas[idx]);
 	lsas[idx] = NULL;
 	return CMD_SUCCESS;
 }