lib: change thread_add_* API

Do not return pointer to the newly created thread from various thread_add
functions. This should prevent developers from storing a thread pointer
into some variable without letting the lib know that the pointer is
stored. When the lib doesn't know that the pointer is stored, it doesn't
prevent rescheduling and it can lead to hard to find bugs. If someone
wants to store the pointer, they should pass a double pointer as the last
argument.

Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>

diff --git a/lib/thread.c b/lib/thread.c
index 71d7798af51afb0c52185d2008570ad4e8606b16..5dbba6363d05d230a7b05bc3c94c6ac7d8ea9fd5 100644 (file)
--- a/lib/thread.c
+++ b/lib/thread.c
@@ -922,10 +922,10 @@ done:
 }
 
 /* Add new read thread. */
-struct thread *_thread_add_read_write(const struct xref_threadsched *xref,
-                                     struct thread_master *m,
-                                     int (*func)(struct thread *),
-                                     void *arg, int fd, struct thread **t_ptr)
+void _thread_add_read_write(const struct xref_threadsched *xref,
+                           struct thread_master *m,
+                           int (*func)(struct thread *), void *arg, int fd,
+                           struct thread **t_ptr)
 {
        int dir = xref->thread_type;
        struct thread *thread = NULL;
@@ -1000,15 +1000,13 @@ struct thread *_thread_add_read_write(const struct xref_threadsched *xref,
 
                AWAKEN(m);
        }
-
-       return thread;
 }
 
-static struct thread *
-_thread_add_timer_timeval(const struct xref_threadsched *xref,
-                         struct thread_master *m, int (*func)(struct thread *),
-                         void *arg, struct timeval *time_relative,
-                         struct thread **t_ptr)
+static void _thread_add_timer_timeval(const struct xref_threadsched *xref,
+                                     struct thread_master *m,
+                                     int (*func)(struct thread *), void *arg,
+                                     struct timeval *time_relative,
+                                     struct thread **t_ptr)
 {
        struct thread *thread;
        struct timeval t;
@@ -1028,7 +1026,7 @@ _thread_add_timer_timeval(const struct xref_threadsched *xref,
        frr_with_mutex(&m->mtx) {
                if (t_ptr && *t_ptr)
                        /* thread is already scheduled; don't reschedule */
-                       return NULL;
+                       return;
 
                thread = thread_get(m, THREAD_TIMER, func, arg, xref);
 
@@ -1048,16 +1046,13 @@ _thread_add_timer_timeval(const struct xref_threadsched *xref,
                if (thread_timer_list_first(&m->timer) == thread)
                        AWAKEN(m);
        }
-
-       return thread;
 }
 
 
 /* Add timer event thread. */
-struct thread *_thread_add_timer(const struct xref_threadsched *xref,
-                                struct thread_master *m,
-                                int (*func)(struct thread *),
-                                void *arg, long timer, struct thread **t_ptr)
+void _thread_add_timer(const struct xref_threadsched *xref,
+                      struct thread_master *m, int (*func)(struct thread *),
+                      void *arg, long timer, struct thread **t_ptr)
 {
        struct timeval trel;
 
@@ -1066,15 +1061,14 @@ struct thread *_thread_add_timer(const struct xref_threadsched *xref,
        trel.tv_sec = timer;
        trel.tv_usec = 0;
 
-       return _thread_add_timer_timeval(xref, m, func, arg, &trel, t_ptr);
+       _thread_add_timer_timeval(xref, m, func, arg, &trel, t_ptr);
 }
 
 /* Add timer event thread with "millisecond" resolution */
-struct thread *_thread_add_timer_msec(const struct xref_threadsched *xref,
-                                     struct thread_master *m,
-                                     int (*func)(struct thread *),
-                                     void *arg, long timer,
-                                     struct thread **t_ptr)
+void _thread_add_timer_msec(const struct xref_threadsched *xref,
+                           struct thread_master *m,
+                           int (*func)(struct thread *), void *arg, long timer,
+                           struct thread **t_ptr)
 {
        struct timeval trel;
 
@@ -1083,24 +1077,21 @@ struct thread *_thread_add_timer_msec(const struct xref_threadsched *xref,
        trel.tv_sec = timer / 1000;
        trel.tv_usec = 1000 * (timer % 1000);
 
-       return _thread_add_timer_timeval(xref, m, func, arg, &trel, t_ptr);
+       _thread_add_timer_timeval(xref, m, func, arg, &trel, t_ptr);
 }
 
 /* Add timer event thread with "timeval" resolution */
-struct thread *_thread_add_timer_tv(const struct xref_threadsched *xref,
-                                   struct thread_master *m,
-                                   int (*func)(struct thread *),
-                                   void *arg, struct timeval *tv,
-                                   struct thread **t_ptr)
+void _thread_add_timer_tv(const struct xref_threadsched *xref,
+                         struct thread_master *m, int (*func)(struct thread *),
+                         void *arg, struct timeval *tv, struct thread **t_ptr)
 {
-       return _thread_add_timer_timeval(xref, m, func, arg, tv, t_ptr);
+       _thread_add_timer_timeval(xref, m, func, arg, tv, t_ptr);
 }
 
 /* Add simple event thread. */
-struct thread *_thread_add_event(const struct xref_threadsched *xref,
-                                struct thread_master *m,
-                                int (*func)(struct thread *),
-                                void *arg, int val, struct thread **t_ptr)
+void _thread_add_event(const struct xref_threadsched *xref,
+                      struct thread_master *m, int (*func)(struct thread *),
+                      void *arg, int val, struct thread **t_ptr)
 {
        struct thread *thread = NULL;
 
@@ -1128,8 +1119,6 @@ struct thread *_thread_add_event(const struct xref_threadsched *xref,
 
                AWAKEN(m);
        }
-
-       return thread;
 }
 
 /* Thread cancellation ------------------------------------------------------ */

diff --git a/lib/thread.h b/lib/thread.h
index c5f0ffbf773daeded9195195091c4ec3236a7f72..39f21da11d332c0fc318acc6de89f1397902eca7 100644 (file)
--- a/lib/thread.h
+++ b/lib/thread.h
@@ -219,26 +219,30 @@ void thread_master_set_name(struct thread_master *master, const char *name);
 extern void thread_master_free(struct thread_master *);
 extern void thread_master_free_unused(struct thread_master *);
 
-extern struct thread *_thread_add_read_write(
-       const struct xref_threadsched *xref, struct thread_master *master,
-       int (*fn)(struct thread *), void *arg, int fd, struct thread **tref);
-
-extern struct thread *_thread_add_timer(
-       const struct xref_threadsched *xref, struct thread_master *master,
-       int (*fn)(struct thread *), void *arg, long t, struct thread **tref);
-
-extern struct thread *_thread_add_timer_msec(
-       const struct xref_threadsched *xref, struct thread_master *master,
-       int (*fn)(struct thread *), void *arg, long t, struct thread **tref);
-
-extern struct thread *_thread_add_timer_tv(
-       const struct xref_threadsched *xref, struct thread_master *master,
-       int (*fn)(struct thread *), void *arg, struct timeval *tv,
-       struct thread **tref);
-
-extern struct thread *_thread_add_event(
-       const struct xref_threadsched *xref, struct thread_master *master,
-       int (*fn)(struct thread *), void *arg, int val, struct thread **tref);
+extern void _thread_add_read_write(const struct xref_threadsched *xref,
+                                  struct thread_master *master,
+                                  int (*fn)(struct thread *), void *arg,
+                                  int fd, struct thread **tref);
+
+extern void _thread_add_timer(const struct xref_threadsched *xref,
+                             struct thread_master *master,
+                             int (*fn)(struct thread *), void *arg, long t,
+                             struct thread **tref);
+
+extern void _thread_add_timer_msec(const struct xref_threadsched *xref,
+                                  struct thread_master *master,
+                                  int (*fn)(struct thread *), void *arg,
+                                  long t, struct thread **tref);
+
+extern void _thread_add_timer_tv(const struct xref_threadsched *xref,
+                                struct thread_master *master,
+                                int (*fn)(struct thread *), void *arg,
+                                struct timeval *tv, struct thread **tref);
+
+extern void _thread_add_event(const struct xref_threadsched *xref,
+                             struct thread_master *master,
+                             int (*fn)(struct thread *), void *arg, int val,
+                             struct thread **tref);
 
 extern void _thread_execute(const struct xref_threadsched *xref,
                            struct thread_master *master,

diff --git a/lib/vty.c b/lib/vty.c
index fef16f1ee7a6e3dab92bbd5eb4ee9a270f5a1d92..11a2fe0d9c409b4bf62a1df2bf8998403d90f3fd 100644 (file)
--- a/lib/vty.c
+++ b/lib/vty.c
@@ -2697,19 +2697,21 @@ static struct thread_master *vty_master;
 
 static void vty_event_serv(enum event event, int sock)
 {
-       struct thread *vty_serv_thread = NULL;
+       struct thread **vty_serv_thread_ptr = NULL;
 
        switch (event) {
        case VTY_SERV:
-               vty_serv_thread = thread_add_read(vty_master, vty_accept,
-                                                 NULL, sock, NULL);
-               vector_set_index(Vvty_serv_thread, sock, vty_serv_thread);
+               vty_serv_thread_ptr = (struct thread **)vector_get_index(
+                       Vvty_serv_thread, sock);
+               thread_add_read(vty_master, vty_accept, NULL, sock,
+                               vty_serv_thread_ptr);
                break;
 #ifdef VTYSH
        case VTYSH_SERV:
-               vty_serv_thread = thread_add_read(vty_master, vtysh_accept,
-                                                 NULL, sock, NULL);
-               vector_set_index(Vvty_serv_thread, sock, vty_serv_thread);
+               vty_serv_thread_ptr = (struct thread **)vector_get_index(
+                       Vvty_serv_thread, sock);
+               thread_add_read(vty_master, vtysh_accept, NULL, sock,
+                               vty_serv_thread_ptr);
                break;
 #endif /* VTYSH */
        default:

diff --git a/ospfd/ospf_lsa.c b/ospfd/ospf_lsa.c
index cc1b2919c0cc298790891db7eda494653682ecfd..10541b09bb8941452cfd20da31d3431b3fd685ed 100644 (file)
--- a/ospfd/ospf_lsa.c
+++ b/ospfd/ospf_lsa.c
@@ -2480,10 +2480,6 @@ ospf_router_lsa_install(struct ospf *ospf, struct ospf_lsa *new, int rt_recalc)
        return new;
 }
 
-#define OSPF_INTERFACE_TIMER_ON(T, F, V)                                       \
-       if (!(T))                                                              \
-       (T) = thread_add_timer(master, (F), oi, (V))
-
 /* Install network-LSA to an area. */
 static struct ospf_lsa *ospf_network_lsa_install(struct ospf *ospf,
                                                 struct ospf_interface *oi,

diff --git a/ospfd/ospf_lsa.h b/ospfd/ospf_lsa.h
index 5dcd0727749ded70ab0e26809bb2aaff0c4daeee..2e648b33543734f816adc568d3a20723be4e6efe 100644 (file)
--- a/ospfd/ospf_lsa.h
+++ b/ospfd/ospf_lsa.h
@@ -223,10 +223,6 @@ struct as_external_lsa {
 
 #define OSPF_LSA_UPDATE_DELAY          2
 
-#define OSPF_LSA_UPDATE_TIMER_ON(T, F)                                         \
-       if (!(T))                                                              \
-       (T) = thread_add_timer(master, (F), 0, 2)
-
 #define CHECK_LSA_TYPE_1_TO_5_OR_7(type)                                       \
        ((type == OSPF_ROUTER_LSA) || (type == OSPF_NETWORK_LSA)               \
         || (type == OSPF_SUMMARY_LSA) || (type == OSPF_ASBR_SUMMARY_LSA)      \

diff --git a/zebra/zebra_netns_notify.c b/zebra/zebra_netns_notify.c
index 61f97ce6a948e061f51356190d9f5f982fb36bc0..2d25801590636a5386c8ab60469886e62ea8a809 100644 (file)
--- a/zebra/zebra_netns_notify.c
+++ b/zebra/zebra_netns_notify.c
@@ -249,8 +249,8 @@ static int zebra_ns_notify_read(struct thread *t)
        char buf[BUFSIZ];
        ssize_t len;
 
-       zebra_netns_notify_current = thread_add_read(
-               zrouter.master, zebra_ns_notify_read, NULL, fd_monitor, NULL);
+       thread_add_read(zrouter.master, zebra_ns_notify_read, NULL, fd_monitor,
+                       &zebra_netns_notify_current);
        len = read(fd_monitor, buf, sizeof(buf));
        if (len < 0) {
                flog_err_sys(EC_ZEBRA_NS_NOTIFY_READ,
@@ -359,8 +359,8 @@ void zebra_ns_notify_init(void)
                             "NS notify watch: failed to add watch (%s)",
                             safe_strerror(errno));
        }
-       zebra_netns_notify_current = thread_add_read(
-               zrouter.master, zebra_ns_notify_read, NULL, fd_monitor, NULL);
+       thread_add_read(zrouter.master, zebra_ns_notify_read, NULL, fd_monitor,
+                       &zebra_netns_notify_current);
 }
 
 void zebra_ns_notify_close(void)