Merge pull request FRRouting#14383 from donaldsharp/bgp_coverity_clea…

…nup_early_sept

Bgp coverity cleanup early sept

bgpd/bgp_advertise.c

@@ -188,22 +188,22 @@ void bgp_adj_in_set(struct bgp_dest *dest, struct peer *peer, struct attr *attr,
 	bgp_dest_lock_node(dest);
 }
 
-void bgp_adj_in_remove(struct bgp_dest *dest, struct bgp_adj_in *bai)
+void bgp_adj_in_remove(struct bgp_dest **dest, struct bgp_adj_in *bai)
 {
 	bgp_attr_unintern(&bai->attr);
-	BGP_ADJ_IN_DEL(dest, bai);
-	bgp_dest_unlock_node(dest);
+	BGP_ADJ_IN_DEL(*dest, bai);
+	*dest = bgp_dest_unlock_node(*dest);
 	peer_unlock(bai->peer); /* adj_in peer reference */
 	XFREE(MTYPE_BGP_ADJ_IN, bai);
 }
 
-bool bgp_adj_in_unset(struct bgp_dest *dest, struct peer *peer,
+bool bgp_adj_in_unset(struct bgp_dest **dest, struct peer *peer,
 		      uint32_t addpath_id)
 {
 	struct bgp_adj_in *adj;
 	struct bgp_adj_in *adj_next;
 
-	adj = dest->adj_in;
+	adj = (*dest)->adj_in;
 
 	if (!adj)
 		return false;
@@ -215,6 +215,8 @@ bool bgp_adj_in_unset(struct bgp_dest *dest, struct peer *peer,
 			bgp_adj_in_remove(dest, adj);
 
 		adj = adj_next;
+
+		assert(*dest);
 	}
 
 	return true;

bgpd/bgp_advertise.h

@@ -130,9 +130,9 @@ extern bool bgp_adj_out_lookup(struct peer *peer, struct bgp_dest *dest,
 			       uint32_t addpath_tx_id);
 extern void bgp_adj_in_set(struct bgp_dest *dest, struct peer *peer,
 			   struct attr *attr, uint32_t addpath_id);
-extern bool bgp_adj_in_unset(struct bgp_dest *dest, struct peer *peer,
+extern bool bgp_adj_in_unset(struct bgp_dest **dest, struct peer *peer,
 			     uint32_t addpath_id);
-extern void bgp_adj_in_remove(struct bgp_dest *dest, struct bgp_adj_in *bai);
+extern void bgp_adj_in_remove(struct bgp_dest **dest, struct bgp_adj_in *bai);
 
 extern unsigned int bgp_advertise_attr_hash_key(const void *p);
 extern bool bgp_advertise_attr_hash_cmp(const void *p1, const void *p2);

bgpd/bgp_evpn.c

@@ -2035,21 +2035,22 @@ static void evpn_zebra_reinstall_best_route(struct bgp *bgp,
  * additional handling to prevent bgp from injecting and holding on to a
  * non-best local path.
  */
-static void evpn_cleanup_local_non_best_route(struct bgp *bgp,
-					      struct bgpevpn *vpn,
-					      struct bgp_dest *dest,
-					      struct bgp_path_info *local_pi)
+static struct bgp_dest *
+evpn_cleanup_local_non_best_route(struct bgp *bgp, struct bgpevpn *vpn,
+				  struct bgp_dest *dest,
+				  struct bgp_path_info *local_pi)
 {
 	/* local path was not picked as the winner; kick it out */
 	if (bgp_debug_zebra(NULL))
 		zlog_debug("evicting local evpn prefix %pBD as remote won",
 			   dest);
 
 	evpn_delete_old_local_route(bgp, vpn, dest, local_pi, NULL);
-	bgp_path_info_reap(dest, local_pi);
 
 	/* tell zebra to re-add the best remote path */
 	evpn_zebra_reinstall_best_route(bgp, vpn, dest);
+
+	return bgp_path_info_reap(dest, local_pi);
 }
 
 static inline bool bgp_evpn_route_add_l3_ecomm_ok(struct bgpevpn *vpn,
@@ -2186,7 +2187,8 @@ static int update_evpn_route(struct bgp *bgp, struct bgpevpn *vpn,
 	} else {
 		if (!CHECK_FLAG(pi->flags, BGP_PATH_SELECTED)) {
 			route_change = 0;
-			evpn_cleanup_local_non_best_route(bgp, vpn, dest, pi);
+			dest = evpn_cleanup_local_non_best_route(bgp, vpn, dest,
+								 pi);
 		} else {
 			bool new_is_sync;
 
@@ -2203,7 +2205,8 @@ static int update_evpn_route(struct bgp *bgp, struct bgpevpn *vpn,
 	}
 	bgp_path_info_unlock(pi);
 
-	bgp_dest_unlock_node(dest);
+	if (dest)
+		bgp_dest_unlock_node(dest);
 
 	/* If this is a new route or some attribute has changed, export the
 	 * route to the global table. The route will be advertised to peers
@@ -2327,9 +2330,13 @@ static int delete_evpn_route(struct bgp *bgp, struct bgpevpn *vpn,
 	 */
 	delete_evpn_route_entry(bgp, afi, safi, dest, &pi);
 	if (pi) {
-		bgp_path_info_reap(dest, pi);
+		dest = bgp_path_info_reap(dest, pi);
+		assert(dest);
 		evpn_route_select_install(bgp, vpn, dest);
 	}
+
+	/* dest should still exist due to locking make coverity happy */
+	assert(dest);
 	bgp_dest_unlock_node(dest);
 
 	return 0;
@@ -2571,7 +2578,8 @@ static void delete_global_type2_routes(struct bgp *bgp, struct bgpevpn *vpn)
 	}
 }
 
-static void delete_vni_type2_route(struct bgp *bgp, struct bgp_dest *dest)
+static struct bgp_dest *delete_vni_type2_route(struct bgp *bgp,
+					       struct bgp_dest *dest)
 {
 	struct bgp_path_info *pi;
 	afi_t afi = AFI_L2VPN;
@@ -2581,13 +2589,15 @@ static void delete_vni_type2_route(struct bgp *bgp, struct bgp_dest *dest)
 		(const struct prefix_evpn *)bgp_dest_get_prefix(dest);
 
 	if (evp->prefix.route_type != BGP_EVPN_MAC_IP_ROUTE)
-		return;
+		return dest;
 
 	delete_evpn_route_entry(bgp, afi, safi, dest, &pi);
 
 	/* Route entry in local table gets deleted immediately. */
 	if (pi)
-		bgp_path_info_reap(dest, pi);
+		dest = bgp_path_info_reap(dest, pi);
+
+	return dest;
 }
 
 static void delete_vni_type2_routes(struct bgp *bgp, struct bgpevpn *vpn)
@@ -2598,12 +2608,16 @@ static void delete_vni_type2_routes(struct bgp *bgp, struct bgpevpn *vpn)
 	 * routes.
 	 */
 	for (dest = bgp_table_top(vpn->mac_table); dest;
-	     dest = bgp_route_next(dest))
-		delete_vni_type2_route(bgp, dest);
+	     dest = bgp_route_next(dest)) {
+		dest = delete_vni_type2_route(bgp, dest);
+		assert(dest);
+	}
 
 	for (dest = bgp_table_top(vpn->ip_table); dest;
-	     dest = bgp_route_next(dest))
-		delete_vni_type2_route(bgp, dest);
+	     dest = bgp_route_next(dest)) {
+		dest = delete_vni_type2_route(bgp, dest);
+		assert(dest);
+	}
 }
 
 /*
@@ -2635,7 +2649,9 @@ static void delete_all_vni_routes(struct bgp *bgp, struct bgpevpn *vpn)
 		     (pi != NULL) && (nextpi = pi->next, 1); pi = nextpi) {
 			bgp_evpn_remote_ip_hash_del(vpn, pi);
 			bgp_path_info_delete(dest, pi);
-			bgp_path_info_reap(dest, pi);
+			dest = bgp_path_info_reap(dest, pi);
+
+			assert(dest);
 		}
 	}
 
@@ -2644,7 +2660,9 @@ static void delete_all_vni_routes(struct bgp *bgp, struct bgpevpn *vpn)
 		for (pi = bgp_dest_get_bgp_path_info(dest);
 		     (pi != NULL) && (nextpi = pi->next, 1); pi = nextpi) {
 			bgp_path_info_delete(dest, pi);
-			bgp_path_info_reap(dest, pi);
+			dest = bgp_path_info_reap(dest, pi);
+
+			assert(dest);
 		}
 	}
 }
@@ -3011,14 +3029,14 @@ static int install_evpn_route_entry_in_vrf(struct bgp *bgp_vrf,
 	/* Process for route leaking. */
 	vpn_leak_from_vrf_update(bgp_get_default(), bgp_vrf, pi);
 
-	bgp_dest_unlock_node(dest);
-
 	if (bgp_debug_zebra(NULL))
 		zlog_debug("... %s pi dest %p (l %d) pi %p (l %d, f 0x%x)",
 			   new_pi ? "new" : "update", dest,
 			   bgp_dest_get_lock_count(dest), pi, pi->lock,
 			   pi->flags);
 
+	bgp_dest_unlock_node(dest);
+
 	return ret;
 }
 

bgpd/bgp_evpn_mh.c

@@ -323,7 +323,9 @@ static void bgp_evpn_es_route_del_all(struct bgp *bgp, struct bgp_evpn_es *es)
 		for (pi = bgp_dest_get_bgp_path_info(dest);
 		     (pi != NULL) && (nextpi = pi->next, 1); pi = nextpi) {
 			bgp_path_info_delete(dest, pi);
-			bgp_path_info_reap(dest, pi);
+			dest = bgp_path_info_reap(dest, pi);
+
+			assert(dest);
 		}
 	}
 }
@@ -513,8 +515,11 @@ static int bgp_evpn_mh_route_delete(struct bgp *bgp, struct bgp_evpn_es *es,
 	 */
 	delete_evpn_route_entry(bgp, afi, safi, dest, &pi);
 	if (pi)
-		bgp_path_info_reap(dest, pi);
+		dest = bgp_path_info_reap(dest, pi);
+
+	assert(dest);
 	bgp_dest_unlock_node(dest);
+
 	return 0;
 }
 

bgpd/bgp_label.c

@@ -195,7 +195,8 @@ int bgp_reg_for_label_callback(mpls_label_t new_label, void *labelid,
 		return -1;
 	}
 
-	bgp_dest_unlock_node(dest);
+	dest = bgp_dest_unlock_node(dest);
+	assert(dest);
 
 	if (BGP_DEBUG(labelpool, LABELPOOL))
 		zlog_debug("%s: FEC %pRN label=%u, allocated=%d", __func__,

bgpd/bgp_mplsvpn.c

@@ -1238,13 +1238,14 @@ leak_update(struct bgp *to_bgp, struct bgp_dest *bn,
 	bgp_aggregate_increment(to_bgp, p, new, afi, safi);
 	bgp_path_info_add(bn, new);
 
-	bgp_dest_unlock_node(bn);
 	bgp_process(to_bgp, bn, afi, safi);
 
 	if (debug)
 		zlog_debug("%s: ->%s: %pBD: Added new route", __func__,
 			   to_bgp->name_pretty, bn);
 
+	bgp_dest_unlock_node(bn);
+
 	return new;
 }
 

bgpd/bgp_nexthop.c

@@ -484,7 +484,9 @@ void bgp_connected_delete(struct bgp *bgp, struct connected *ifc)
 		XFREE(MTYPE_BGP_CONN, bc);
 		bgp_dest_set_bgp_connected_ref_info(dest, NULL);
 	}
-	bgp_dest_unlock_node(dest);
+
+	dest = bgp_dest_unlock_node(dest);
+	assert(dest);
 	bgp_dest_unlock_node(dest);
 }