From: Donald Sharp <sharpd@cumulusnetworks.com>
Date: Thu, 28 Sep 2017 01:19:20 +0000 (-0400)
Subject: *: Convert list_free usage to list_delete
X-Git-Tag: frr-4.0-dev~237^2
X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=acdf5e25101bafe334e6b500c3dd0a2babb3c1ec;p=matthieu%2Ffrr.git

*: Convert list_free usage to list_delete

list_free is occassionally being used to delete the
list and accidently not deleting all the nodes.
We keep running across this usage pattern.  Let's
remove the temptation and only allow list_delete
to handle list deletion.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
---

diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c
index f5c0fd6010..a09d966d7b 100644
--- a/bgpd/bgp_evpn.c
+++ b/bgpd/bgp_evpn.c
@@ -288,7 +288,7 @@ static void unmap_vni_from_rt(struct bgp *bgp, struct bgpevpn *vpn,
 	/* Delete VNI from hash list for this RT. */
 	listnode_delete(irt->vnis, vpn);
 	if (!listnode_head(irt->vnis)) {
-		list_free(irt->vnis);
+		list_delete_and_null(&irt->vnis);
 		import_rt_free(bgp, irt);
 	}
 }
diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c
index bc69b67de9..c90257d659 100644
--- a/bgpd/bgp_zebra.c
+++ b/bgpd/bgp_zebra.c
@@ -1290,10 +1290,8 @@ static void bgp_redist_del(struct bgp *bgp, afi_t afi, u_char type,
 	if (red) {
 		listnode_delete(bgp->redist[afi][type], red);
 		XFREE(MTYPE_BGP_REDIST, red);
-		if (!bgp->redist[afi][type]->count) {
-			list_free(bgp->redist[afi][type]);
-			bgp->redist[afi][type] = NULL;
-		}
+		if (!bgp->redist[afi][type]->count)
+			list_delete_and_null(&bgp->redist[afi][type]);
 	}
 }
 
diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index 28f0b14d39..308698e1c5 100644
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -7454,8 +7454,7 @@ void bgp_terminate(void)
 	/* reverse bgp_master_init */
 	bgp_close();
 	if (bm->listen_sockets)
-		list_free(bm->listen_sockets);
-	bm->listen_sockets = NULL;
+		list_delete_and_null(&bm->listen_sockets);
 
 	for (ALL_LIST_ELEMENTS(bm->bgp, mnode, mnnode, bgp))
 		for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer))
diff --git a/bgpd/rfapi/vnc_export_bgp.c b/bgpd/rfapi/vnc_export_bgp.c
index b699cec9e7..75347a7eea 100644
--- a/bgpd/rfapi/vnc_export_bgp.c
+++ b/bgpd/rfapi/vnc_export_bgp.c
@@ -1544,7 +1544,7 @@ void vnc_direct_bgp_vpn_disable(struct bgp *bgp, afi_t afi)
 		if (nve_list) {
 			vnc_direct_bgp_unexport_table(
 				afi, it->imported_vpn[afi], nve_list);
-			list_free(nve_list);
+			list_delete_and_null(&nve_list);
 		}
 	}
 }
diff --git a/eigrpd/eigrp_topology.c b/eigrpd/eigrp_topology.c
index e8532a077c..94775622d9 100644
--- a/eigrpd/eigrp_topology.c
+++ b/eigrpd/eigrp_topology.c
@@ -101,8 +101,7 @@ static int eigrp_prefix_entry_cmp(struct eigrp_prefix_entry *node1,
 
 static void eigrp_prefix_entry_del(struct eigrp_prefix_entry *node)
 {
-	list_delete_all_node(node->entries);
-	list_free(node->entries);
+	list_delete_and_null(&node->entries);
 }
 
 /*
@@ -158,7 +157,7 @@ struct eigrp_nexthop_entry *eigrp_nexthop_entry_new()
  */
 void eigrp_topology_free(struct list *list)
 {
-	list_free(list);
+	list_delete_and_null(&list);
 }
 
 /*
@@ -217,9 +216,8 @@ void eigrp_prefix_entry_delete(struct list *topology,
 	listnode_delete(eigrp->topology_changes_internalIPV4, node);
 
 	if (listnode_lookup(topology, node) != NULL) {
-		list_delete_all_node(node->entries);
-		list_free(node->entries);
-		list_free(node->rij);
+		list_delete_and_null(&node->entries);
+		list_delete_and_null(&node->rij);
 		listnode_delete(topology, node);
 		eigrp_zebra_route_delete(node->destination);
 		XFREE(MTYPE_EIGRP_PREFIX_ENTRY, node);
diff --git a/lib/if.c b/lib/if.c
index ce5816fd22..34de2c8d8b 100644
--- a/lib/if.c
+++ b/lib/if.c
@@ -191,8 +191,8 @@ void if_delete(struct interface *ifp)
 
 	if_delete_retain(ifp);
 
-	list_free(ifp->connected);
-	list_free(ifp->nbr_connected);
+	list_delete_and_null(&ifp->connected);
+	list_delete_and_null(&ifp->nbr_connected);
 
 	if_link_params_free(ifp);
 
diff --git a/lib/linklist.c b/lib/linklist.c
index 2f9e7c1e33..2306dd6d00 100644
--- a/lib/linklist.c
+++ b/lib/linklist.c
@@ -33,7 +33,7 @@ struct list *list_new(void)
 }
 
 /* Free list. */
-void list_free(struct list *l)
+static void list_free_internal(struct list *l)
 {
 	XFREE(MTYPE_LINK_LIST, l);
 }
@@ -252,7 +252,7 @@ void list_delete_and_null(struct list **list)
 {
 	assert(*list);
 	list_delete_all_node(*list);
-	list_free(*list);
+	list_free_internal(*list);
 	*list = NULL;
 }
 
diff --git a/lib/linklist.h b/lib/linklist.h
index 5ae728ce0e..4a65fead86 100644
--- a/lib/linklist.h
+++ b/lib/linklist.h
@@ -61,7 +61,6 @@ struct list {
 /* Prototypes. */
 extern struct list *
 list_new(void); /* encouraged: set list.del callback on new lists */
-extern void list_free(struct list *);
 
 extern void listnode_add(struct list *, void *);
 extern void listnode_add_sort(struct list *, void *);
@@ -77,16 +76,22 @@ extern void *listnode_head(struct list *);
 /*
  * The usage of list_delete is being transitioned to pass in
  * the double pointer to remove use after free's.
+ * list_free usage is deprecated, it leads to memory leaks
+ * of the linklist nodes.  Please use list_delete_and_null
+ *
  * In Oct of 2018, rename list_delete_and_null to list_delete
  * and remove list_delete_original and the list_delete #define
+ * Additionally remove list_free entirely
  */
 #if CONFDATE > 20181001
 CPP_NOTICE("list_delete without double pointer is deprecated, please fixup")
 #endif
 extern void list_delete_and_null(struct list **);
 extern void list_delete_original(struct list *);
-#define list_delete(X) list_delete_original((X))  \
+#define list_delete(X) list_delete_original((X))			\
 	CPP_WARN("Please transition to using list_delete_and_null")
+#define list_free(X) list_delete_original((X))				\
+	CPP_WARN("Please transition tousing list_delete_and_null")
 
 extern void list_delete_all_node(struct list *);
 
diff --git a/lib/thread.c b/lib/thread.c
index fce8dee164..2d37857b8a 100644
--- a/lib/thread.c
+++ b/lib/thread.c
@@ -554,8 +554,7 @@ void thread_master_free(struct thread_master *m)
 	{
 		listnode_delete(masters, m);
 		if (masters->count == 0) {
-			list_free(masters);
-			masters = NULL;
+			list_delete_and_null(&masters);
 		}
 	}
 	pthread_mutex_unlock(&masters_mtx);
diff --git a/lib/zclient.c b/lib/zclient.c
index 0b06dbacba..43d46a1801 100644
--- a/lib/zclient.c
+++ b/lib/zclient.c
@@ -123,8 +123,7 @@ void redist_del_instance(struct redist_proto *red, u_short instance)
 	XFREE(MTYPE_REDIST_INST, id);
 	if (!red->instances->count) {
 		red->enabled = 0;
-		list_free(red->instances);
-		red->instances = NULL;
+		list_delete_and_null(&red->instances);
 	}
 }
 
diff --git a/ospfd/ospf_ase.c b/ospfd/ospf_ase.c
index 1abb1a6a3f..2f1b27f0f1 100644
--- a/ospfd/ospf_ase.c
+++ b/ospfd/ospf_ase.c
@@ -77,7 +77,7 @@ struct ospf_route *ospf_find_asbr_route(struct ospf *ospf,
 
 	/* If none is found -- look through all. */
 	if (listcount(chosen) == 0) {
-		list_free(chosen);
+		list_delete_and_null(&chosen);
 		chosen = rn->info;
 	}
 
diff --git a/ospfd/ospf_interface.c b/ospfd/ospf_interface.c
index b57f8a4d38..67ce6f1713 100644
--- a/ospfd/ospf_interface.c
+++ b/ospfd/ospf_interface.c
@@ -313,10 +313,10 @@ void ospf_if_free(struct ospf_interface *oi)
 	route_table_finish(oi->ls_upd_queue);
 
 	/* Free any lists that should be freed */
-	list_free(oi->nbr_nbma);
+	list_delete_and_null(&oi->nbr_nbma);
 
-	list_free(oi->ls_ack);
-	list_free(oi->ls_ack_direct.ls_ack);
+	list_delete_and_null(&oi->ls_ack);
+	list_delete_and_null(&oi->ls_ack_direct.ls_ack);
 
 	if (IS_DEBUG_OSPF_EVENT)
 		zlog_debug("%s: ospf interface %s vrf %s id %u deleted",
diff --git a/ospfd/ospf_lsa.c b/ospfd/ospf_lsa.c
index 270021cab0..1795225ca7 100644
--- a/ospfd/ospf_lsa.c
+++ b/ospfd/ospf_lsa.c
@@ -3622,7 +3622,7 @@ void ospf_refresher_unregister_lsa(struct ospf *ospf, struct ospf_lsa *lsa)
 			ospf->lsa_refresh_queue.qs[lsa->refresh_list];
 		listnode_delete(refresh_list, lsa);
 		if (!listcount(refresh_list)) {
-			list_free(refresh_list);
+			list_delete_and_null(&refresh_list);
 			ospf->lsa_refresh_queue.qs[lsa->refresh_list] = NULL;
 		}
 		ospf_lsa_unlock(&lsa); /* lsa_refresh_queue */
@@ -3691,7 +3691,7 @@ int ospf_lsa_refresh_walker(struct thread *t)
 				lsa->refresh_list = -1;
 				listnode_add(lsa_to_refresh, lsa);
 			}
-			list_free(refresh_list);
+			list_delete_and_null(&refresh_list);
 		}
 	}
 
diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
index 36583a63ee..47f5ee76d2 100644
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -1617,7 +1617,7 @@ static void ospf_ls_req(struct ip *iph, struct ospf_header *ospfh,
 
 		list_delete_and_null(&ls_upd);
 	} else
-		list_free(ls_upd);
+		list_delete_and_null(&ls_upd);
 }
 
 /* Get the list of LSAs from Link State Update packet.
diff --git a/ospfd/ospf_zebra.c b/ospfd/ospf_zebra.c
index d8fd9afbd7..bd944ae748 100644
--- a/ospfd/ospf_zebra.c
+++ b/ospfd/ospf_zebra.c
@@ -574,8 +574,7 @@ void ospf_external_del(u_char type, u_short instance)
 
 		listnode_delete(om->external[type], ext);
 		if (!om->external[type]->count) {
-			list_free(om->external[type]);
-			om->external[type] = NULL;
+			list_delete_and_null(&om->external[type]);
 		}
 		XFREE(MTYPE_OSPF_EXTERNAL, ext);
 	}
@@ -633,8 +632,7 @@ void ospf_redist_del(struct ospf *ospf, u_char type, u_short instance)
 	if (red) {
 		listnode_delete(ospf->redist[type], red);
 		if (!ospf->redist[type]->count) {
-			list_free(ospf->redist[type]);
-			ospf->redist[type] = NULL;
+			list_delete_and_null(&ospf->redist[type]);
 		}
 		ospf_routemap_unset(red);
 		XFREE(MTYPE_OSPF_REDISTRIBUTE, red);
diff --git a/ripd/ripd.c b/ripd/ripd.c
index 2a0752253f..bededba7fc 100644
--- a/ripd/ripd.c
+++ b/ripd/ripd.c
@@ -132,8 +132,7 @@ static int rip_garbage_collect(struct thread *t)
 	/* Unlock route_node. */
 	listnode_delete(rp->info, rinfo);
 	if (list_isempty((struct list *)rp->info)) {
-		list_free(rp->info);
-		rp->info = NULL;
+		list_delete_and_null((struct list **)&rp->info);
 		route_unlock_node(rp);
 	}
 
diff --git a/ripngd/ripngd.c b/ripngd/ripngd.c
index 1e42315028..0bee9a8bf4 100644
--- a/ripngd/ripngd.c
+++ b/ripngd/ripngd.c
@@ -407,8 +407,7 @@ static int ripng_garbage_collect(struct thread *t)
 	/* Unlock route_node. */
 	listnode_delete(rp->info, rinfo);
 	if (list_isempty((struct list *)rp->info)) {
-		list_free(rp->info);
-		rp->info = NULL;
+		list_delete_and_null((struct list **)&rp->info);
 		route_unlock_node(rp);
 	}
 
@@ -2150,7 +2149,7 @@ DEFUN (clear_ipv6_rip,
 		}
 
 		if (list_isempty(list)) {
-			list_free(list);
+			list_delete_and_null(&list);
 			rp->info = NULL;
 			route_unlock_node(rp);
 		}
diff --git a/zebra/interface.c b/zebra/interface.c
index 9a9f76eab1..a65dd21f63 100644
--- a/zebra/interface.c
+++ b/zebra/interface.c
@@ -138,7 +138,7 @@ static int if_zebra_delete_hook(struct interface *ifp)
 		struct rtadvconf *rtadv;
 
 		rtadv = &zebra_if->rtadv;
-		list_free(rtadv->AdvPrefixList);
+		list_delete_and_null(&rtadv->AdvPrefixList);
 #endif /* HAVE_RTADV */
 
 		XFREE(MTYPE_TMP, zebra_if);
@@ -322,7 +322,7 @@ int if_subnet_delete(struct interface *ifp, struct connected *ifc)
 	}
 
 	/* Otherwise, free list and route node. */
-	list_free(addr_list);
+	list_delete_and_null(&addr_list);
 	rn->info = NULL;
 	route_unlock_node(rn);
 
diff --git a/zebra/zebra_mpls.c b/zebra/zebra_mpls.c
index 9fd07d22d8..7b87355ed4 100644
--- a/zebra/zebra_mpls.c
+++ b/zebra/zebra_mpls.c
@@ -554,7 +554,7 @@ static zebra_fec_t *fec_add(struct route_table *table, struct prefix *p,
  */
 static int fec_del(zebra_fec_t *fec)
 {
-	list_free(fec->client_list);
+	list_delete_and_null(&fec->client_list);
 	fec->rn->info = NULL;
 	route_unlock_node(fec->rn);
 	XFREE(MTYPE_FEC, fec);
diff --git a/zebra/zebra_rnh.c b/zebra/zebra_rnh.c
index fabb4f9e14..355fef94f4 100644
--- a/zebra/zebra_rnh.c
+++ b/zebra/zebra_rnh.c
@@ -160,9 +160,9 @@ struct rnh *zebra_lookup_rnh(struct prefix *p, vrf_id_t vrfid, rnh_type_t type)
 void zebra_free_rnh(struct rnh *rnh)
 {
 	rnh->flags |= ZEBRA_NHT_DELETED;
-	list_free(rnh->client_list);
-	list_free(rnh->zebra_static_route_list);
-	list_free(rnh->zebra_pseudowire_list);
+	list_delete_and_null(&rnh->client_list);
+	list_delete_and_null(&rnh->zebra_static_route_list);
+	list_delete_and_null(&rnh->zebra_pseudowire_list);
 	free_state(rnh->vrf_id, rnh->state, rnh->node);
 	XFREE(MTYPE_RNH, rnh);
 }