From 43b8ca995b8ae5ea0e08ae74cdc0cf68cfa0feef Mon Sep 17 00:00:00 2001
From: Donald Sharp <sharpd@cumulusnetworks.com>
Date: Thu, 13 Jun 2019 19:30:51 -0400
Subject: [PATCH] lib: Do not blindly set the _read and _write pointers to NULL

Adding a read with the address of the thread pointer we want to
use will allow lib/thread.c to properly handle your thread pointers.
Instead we were setting the pointer to NULL before we passed
into the _read and _write thread functions.  Remove the NULL
pointer set and just let thread.c handle everything.

vty_stdio_resume and vty_read would blindly add read and write
which would cause vty_event() to drop the thread pointer.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
---
 lib/vty.c | 48 ++++++++++++------------------------------------
 1 file changed, 12 insertions(+), 36 deletions(-)

diff --git a/lib/vty.c b/lib/vty.c
index 0db3dc36f2..18a449f647 100644
--- a/lib/vty.c
+++ b/lib/vty.c
@@ -1335,7 +1335,6 @@ static int vty_read(struct thread *thread)
 
 	int vty_sock = THREAD_FD(thread);
 	struct vty *vty = THREAD_ARG(thread);
-	vty->t_read = NULL;
 
 	/* Read raw data from socket */
 	if ((nbytes = read(vty->fd, buf, VTY_READ_BUFSIZ)) <= 0) {
@@ -1529,13 +1528,9 @@ static int vty_flush(struct thread *thread)
 	int vty_sock = THREAD_FD(thread);
 	struct vty *vty = THREAD_ARG(thread);
 
-	vty->t_write = NULL;
-
 	/* Tempolary disable read thread. */
-	if ((vty->lines == 0) && vty->t_read) {
-		thread_cancel(vty->t_read);
-		vty->t_read = NULL;
-	}
+	if (vty->lines == 0)
+		THREAD_OFF(vty->t_read);
 
 	/* Function execution continue. */
 	erase = ((vty->status == VTY_MORE || vty->status == VTY_MORELINE));
@@ -1713,12 +1708,9 @@ void vty_stdio_suspend(void)
 	if (!stdio_vty)
 		return;
 
-	if (stdio_vty->t_write)
-		thread_cancel(stdio_vty->t_write);
-	if (stdio_vty->t_read)
-		thread_cancel(stdio_vty->t_read);
-	if (stdio_vty->t_timeout)
-		thread_cancel(stdio_vty->t_timeout);
+	THREAD_OFF(stdio_vty->t_write);
+	THREAD_OFF(stdio_vty->t_read);
+	THREAD_OFF(stdio_vty->t_timeout);
 
 	if (stdio_termios)
 		tcsetattr(0, TCSANOW, &stdio_orig_termios);
@@ -2077,7 +2069,6 @@ static int vtysh_read(struct thread *thread)
 
 	sock = THREAD_FD(thread);
 	vty = THREAD_ARG(thread);
-	vty->t_read = NULL;
 
 	if ((nbytes = read(sock, buf, VTY_READ_BUFSIZ)) <= 0) {
 		if (nbytes < 0) {
@@ -2157,7 +2148,6 @@ static int vtysh_write(struct thread *thread)
 {
 	struct vty *vty = THREAD_ARG(thread);
 
-	vty->t_write = NULL;
 	vtysh_flush(vty);
 	return 0;
 }
@@ -2193,12 +2183,9 @@ void vty_close(struct vty *vty)
 	bool was_stdio = false;
 
 	/* Cancel threads.*/
-	if (vty->t_read)
-		thread_cancel(vty->t_read);
-	if (vty->t_write)
-		thread_cancel(vty->t_write);
-	if (vty->t_timeout)
-		thread_cancel(vty->t_timeout);
+	THREAD_OFF(vty->t_read);
+	THREAD_OFF(vty->t_write);
+	THREAD_OFF(vty->t_timeout);
 
 	/* Flush buffer. */
 	buffer_flush_all(vty->obuf, vty->wfd);
@@ -2254,7 +2241,6 @@ static int vty_timeout(struct thread *thread)
 	struct vty *vty;
 
 	vty = THREAD_ARG(thread);
-	vty->t_timeout = NULL;
 	vty->v_timeout = 0;
 
 	/* Clear buffer*/
@@ -2651,25 +2637,20 @@ static void vty_event(enum event event, int sock, struct vty *vty)
 		vector_set_index(Vvty_serv_thread, sock, vty_serv_thread);
 		break;
 	case VTYSH_READ:
-		vty->t_read = NULL;
 		thread_add_read(vty_master, vtysh_read, vty, sock,
 				&vty->t_read);
 		break;
 	case VTYSH_WRITE:
-		vty->t_write = NULL;
 		thread_add_write(vty_master, vtysh_write, vty, sock,
 				 &vty->t_write);
 		break;
 #endif /* VTYSH */
 	case VTY_READ:
-		vty->t_read = NULL;
 		thread_add_read(vty_master, vty_read, vty, sock, &vty->t_read);
 
 		/* Time out treatment. */
 		if (vty->v_timeout) {
-			if (vty->t_timeout)
-				thread_cancel(vty->t_timeout);
-			vty->t_timeout = NULL;
+			THREAD_OFF(vty->t_timeout);
 			thread_add_timer(vty_master, vty_timeout, vty,
 					 vty->v_timeout, &vty->t_timeout);
 		}
@@ -2679,15 +2660,10 @@ static void vty_event(enum event event, int sock, struct vty *vty)
 				 &vty->t_write);
 		break;
 	case VTY_TIMEOUT_RESET:
-		if (vty->t_timeout) {
-			thread_cancel(vty->t_timeout);
-			vty->t_timeout = NULL;
-		}
-		if (vty->v_timeout) {
-			vty->t_timeout = NULL;
+		THREAD_OFF(vty->t_timeout);
+		if (vty->v_timeout)
 			thread_add_timer(vty_master, vty_timeout, vty,
 					 vty->v_timeout, &vty->t_timeout);
-		}
 		break;
 	}
 }
@@ -3024,7 +3000,7 @@ void vty_reset(void)
 	for (i = 0; i < vector_active(Vvty_serv_thread); i++)
 		if ((vty_serv_thread = vector_slot(Vvty_serv_thread, i))
 		    != NULL) {
-			thread_cancel(vty_serv_thread);
+			THREAD_OFF(vty_serv_thread);
 			vector_slot(Vvty_serv_thread, i) = NULL;
 			close(i);
 		}