*: Convert list_delete(struct list *) to ** to allow nulling

Convert the list_delete(struct list *) function to use
struct list **.  This is to allow the list pointer to be nulled.

I keep running into uses of this list_delete function where we
forget to set the returned pointer to NULL and attempt to use
it and then experience a crash, usually after the developer
has long since left the building.

Let's make the api explicit in it setting the list pointer
to null.

Cynical Prediction:  This code will expose a attempt
to use the NULL'ed list pointer in some obscure bit
of code.

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

10 files changed, 42 insertions, 25 deletions

diff --git a/lib/command.c b/lib/command.c
index d1b086737..97eba96c3 100644
--- a/lib/command.c
+++ b/lib/command.c
@@ -686,7 +686,7 @@ static vector cmd_complete_command_real(vector vline, struct vty *vty,
 	}
 
 	vector comps = completions_to_vec(completions);
-	list_delete(completions);
+	list_delete_and_null(&completions);
 
 	// set status code appropriately
 	switch (vector_active(comps)) {
@@ -1006,7 +1006,7 @@ static int cmd_execute_command_real(vector vline, enum filter_type filter,
 	// if matcher error, return corresponding CMD_ERR
 	if (MATCHER_ERROR(status)) {
 		if (argv_list)
-			list_delete(argv_list);
+			list_delete_and_null(&argv_list);
 		switch (status) {
 		case MATCHER_INCOMPLETE:
 			return CMD_ERR_INCOMPLETE;
@@ -1035,7 +1035,7 @@ static int cmd_execute_command_real(vector vline, enum filter_type filter,
 		ret = matched_element->func(matched_element, vty, argc, argv);
 
 	// delete list and cmd_token's in it
-	list_delete(argv_list);
+	list_delete_and_null(&argv_list);
 	XFREE(MTYPE_TMP, argv);
 
 	return ret;
@@ -2730,6 +2730,6 @@ void cmd_terminate()
 	if (host.config)
 		XFREE(MTYPE_HOST, host.config);
 
-	list_delete(varhandlers);
+	list_delete_and_null(&varhandlers);
 	qobj_finish();
 }
diff --git a/lib/command_match.c b/lib/command_match.c
index 6384abe5c..c60373f91 100644
--- a/lib/command_match.c
+++ b/lib/command_match.c
@@ -333,7 +333,7 @@ static enum matcher_rv command_match_r(struct graph_node *start, vector vline,
 		status = MATCHER_INCOMPLETE;
 
 	// cleanup
-	list_delete(next);
+	list_delete_and_null(&next);
 
 	return status;
 }
@@ -366,7 +366,7 @@ enum matcher_rv command_complete(struct graph *graph, vector vline,
 
 	unsigned int idx;
 	for (idx = 0; idx < vector_active(vline) && next->count > 0; idx++) {
-		list_delete(current);
+		list_delete_and_null(&current);
 		current = next;
 		next = list_new();
 		next->del = stack_del;
@@ -457,8 +457,8 @@ enum matcher_rv command_complete(struct graph *graph, vector vline,
 		}
 	}
 
-	list_delete(current);
-	list_delete(next);
+	list_delete_and_null(&current);
+	list_delete_and_null(&next);
 
 	return mrv;
 }
@@ -648,7 +648,7 @@ static void del_arglist(struct list *list)
 	list_delete_node(list, tail);
 
 	// delete the rest of the list as usual
-	list_delete(list);
+	list_delete_and_null(&list);
 }
 
 /*---------- token level matching functions ----------*/
diff --git a/lib/grammar_sandbox.c b/lib/grammar_sandbox.c
index 3c6396f34..66b042ad9 100644
--- a/lib/grammar_sandbox.c
+++ b/lib/grammar_sandbox.c
@@ -141,7 +141,7 @@ DEFUN (grammar_test_complete,
 		vty_out(vty, "%% No match\n");
 
 	// free resources
-	list_delete(completions);
+	list_delete_and_null(&completions);
 	cmd_free_strvec(command);
 	XFREE(MTYPE_TMP, cmdstr);
 
@@ -185,7 +185,7 @@ DEFUN (grammar_test_match,
 
 		vty_out(vty, "func: %p\n", element->func);
 
-		list_delete(argvv);
+		list_delete_and_null(&argvv);
 	} else {
 		assert(MATCHER_ERROR(result));
 		switch (result) {
@@ -426,7 +426,7 @@ DEFUN (grammar_findambig,
 			}
 			prev = cur;
 		}
-		list_delete(commands);
+		list_delete_and_null(&commands);
 
 		vty_out(vty, "\n");
 	} while (scan && scannode < LINK_PARAMS_NODE);
diff --git a/lib/hash.c b/lib/hash.c
index 243521bef..d2846d737 100644
--- a/lib/hash.c
+++ b/lib/hash.c
@@ -318,8 +318,7 @@ void hash_free(struct hash *hash)
 		if (_hashes) {
 			listnode_delete(_hashes, hash);
 			if (_hashes->count == 0) {
-				list_delete(_hashes);
-				_hashes = NULL;
+				list_delete_and_null(&_hashes);
 			}
 		}
 	}
diff --git a/lib/if.c b/lib/if.c
index 7d2722965..ce5816fd2 100644
--- a/lib/if.c
+++ b/lib/if.c
@@ -1095,8 +1095,7 @@ void if_terminate(struct list **intf_list)
 		if_delete(ifp);
 	}
 
-	list_delete(*intf_list);
-	*intf_list = NULL;
+	list_delete_and_null(intf_list);
 }
 
 const char *if_link_type_str(enum zebra_link_type llt)
diff --git a/lib/keychain.c b/lib/keychain.c
index f0108e080..23a2d72b1 100644
--- a/lib/keychain.c
+++ b/lib/keychain.c
@@ -119,7 +119,7 @@ static void keychain_delete(struct keychain *keychain)
 	if (keychain->name)
 		XFREE(MTYPE_KEYCHAIN, keychain->name);
 
-	list_delete(keychain->key);
+	list_delete_and_null(&keychain->key);
 	listnode_delete(keychain_list, keychain);
 	keychain_free(keychain);
 }
diff --git a/lib/linklist.c b/lib/linklist.c
index c1b056d73..2f9e7c1e3 100644
--- a/lib/linklist.c
+++ b/lib/linklist.c
@@ -239,7 +239,7 @@ void list_delete_all_node(struct list *list)
 	assert(list);
 	for (node = list->head; node; node = next) {
 		next = node->next;
-		if (list->del)
+		if (*list->del)
 			(*list->del)(node->data);
 		listnode_free(node);
 	}
@@ -248,11 +248,17 @@ void list_delete_all_node(struct list *list)
 }
 
 /* Delete all listnode then free list itself. */
-void list_delete(struct list *list)
+void list_delete_and_null(struct list **list)
 {
-	assert(list);
-	list_delete_all_node(list);
-	list_free(list);
+	assert(*list);
+	list_delete_all_node(*list);
+	list_free(*list);
+	*list = NULL;
+}
+
+void list_delete_original(struct list *list)
+{
+	list_delete_and_null(&list);
 }
 
 /* Lookup the node which has given data. */
diff --git a/lib/linklist.h b/lib/linklist.h
index 9bd6e3849..5ae728ce0 100644
--- a/lib/linklist.h
+++ b/lib/linklist.h
@@ -74,7 +74,20 @@ extern void listnode_delete(struct list *, void *);
 extern struct listnode *listnode_lookup(struct list *, void *);
 extern void *listnode_head(struct list *);
 
-extern void list_delete(struct list *);
+/*
+ * The usage of list_delete is being transitioned to pass in
+ * the double pointer to remove use after free's.
+ * In Oct of 2018, rename list_delete_and_null to list_delete
+ * and remove list_delete_original and the list_delete #define
+ */
+#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))  \
+	CPP_WARN("Please transition to using list_delete_and_null")
+
 extern void list_delete_all_node(struct list *);
 
 /* For ospfd and ospf6d. */
diff --git a/lib/thread.c b/lib/thread.c
index a69bd2f0d..fce8dee16 100644
--- a/lib/thread.c
+++ b/lib/thread.c
@@ -570,7 +570,7 @@ void thread_master_free(struct thread_master *m)
 	pthread_cond_destroy(&m->cancel_cond);
 	close(m->io_pipe[0]);
 	close(m->io_pipe[1]);
-	list_delete(m->cancel_req);
+	list_delete_and_null(&m->cancel_req);
 	m->cancel_req = NULL;
 
 	hash_clean(m->cpu_record, cpu_record_hash_free);
diff --git a/lib/wheel.c b/lib/wheel.c
index 9f1f189b7..b1a3e89fc 100644
--- a/lib/wheel.c
+++ b/lib/wheel.c
@@ -99,7 +99,7 @@ void wheel_delete(struct timer_wheel *wheel)
 	int i;
 
 	for (i = 0; i < wheel->slots; i++) {
-		list_delete(wheel->wheel_slot_lists[i]);
+		list_delete_and_null(&wheel->wheel_slot_lists[i]);
 	}
 
 	THREAD_OFF(wheel->timer);