*: 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 f5c0fd60103be8821dc6e689f05918219334bb87..a09d966d7b0086f8f925060dee45450f8e8d608a 100644 (file)
--- 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 bc69b67de9e8d427a9975fe66d55488ede5c8ec2..c90257d6598583103fc7ea159421653eeca360db 100644 (file)
--- 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 28f0b14d397eedf7f786c78233d15ce498e07d27..308698e1c539e77b338b3a6628f49975d886d9a2 100644 (file)
--- 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 b699cec9e7d5b2be750ced9a61e6e1f13ec5fc9e..75347a7eeac6508e91ad0a97625c0436dd847c6a 100644 (file)
--- 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 e8532a077c83ed76c4d26fd9559f1d1be29baee2..94775622d98d02d144d3c6eae26b983810f19df7 100644 (file)
--- 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 ce5816fd22630129a68ea60db770a4c1da76a48c..34de2c8d8b0fea29dab25903d2b7887f9399a88b 100644 (file)
--- 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 2f9e7c1e33db281b859459d329e1adbfb9d9c7ac..2306dd6d0079a5d5ce10ff1f7dd0a6376152761e 100644 (file)
--- 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 5ae728ce0eb5f2645dde4002d845aa4692af9f13..4a65fead861d0ddffab80ef5f0d7708c6aa5b0bf 100644 (file)
--- 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 fce8dee1641d97a908fe050396c33087f9ac8abb..2d37857b8a28903bfa2905b76d0170ef12ea489d 100644 (file)
--- 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 0b06dbacbaf4c683067d9d6b97c3158c17075da0..43d46a180197db23bd7c240a858d559341b91637 100644 (file)
--- 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 1abb1a6a3f6a9b73477ea694739ec4eb3252cfc8..2f1b27f0f1966e0a1e85e07af59d9939b178b56e 100644 (file)
--- 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 b57f8a4d38f990ad22fcb2102595ee89505a52d1..67ce6f17135ec232ffbdf15c693ee354e79d4a18 100644 (file)
--- 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 270021cab0e71d33fae4ae5572dd33a977e3163f..1795225ca7d51d0557a78434b85e37ff8e44e38d 100644 (file)
--- 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 36583a63eee49cb3641b775170478b3541361411..47f5ee76d22e9f5720f440b825f67710455dfc6f 100644 (file)
--- 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 d8fd9afbd7b936e2aaa7d5c39c55a1a4a61287a8..bd944ae748868a6778d157b414bbc2113d728332 100644 (file)
--- 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 2a0752253f4b162812f4ea3c7bb0663db026596d..bededba7fc65eb9973597e28a2b180b7f887e3e0 100644 (file)
--- 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 1e423150285b3d08adead6152f0c20d014211c15..0bee9a8bf494559cad8b99f8920b6c00db49c726 100644 (file)
--- 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 9a9f76eab103f32d19119f653a0c131d07fa46d5..a65dd21f63cfd5ca9d715802637bb89a0cc20d70 100644 (file)
--- 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 9fd07d22d88ba4afb13484960e0a75c1ed77f9c7..7b87355ed4824bb9bdae5042f897718d586377a4 100644 (file)
--- 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 fabb4f9e14d2243d6934fb6fe0c0da2492cc9da5..355fef94f42608d834f83ba4c05a859e68b0aaff 100644 (file)
--- 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);
 }