*: remove THREAD_ON macros, add nullity check

The way thread.c is written, a caller who wishes to be able to cancel a
thread or avoid scheduling it twice must keep a reference to the thread.
Typically this is done with a long lived pointer whose value is checked
for null in order to know if the thread is currently scheduled.  The
check-and-schedule idiom is so common that several wrapper macros in
thread.h existed solely to provide it.

This patch removes those macros and adds a new parameter to all
thread_add_* functions which is a pointer to the struct thread * to
store the result of a scheduling call. If the value passed is non-null,
the thread will only be scheduled if the value is null. This helps with
consistency.

A Coccinelle spatch has been used to transform code of the form:

  if (t == NULL)
    t = thread_add_* (...)

to the form

  thread_add_* (..., &t)

The THREAD_ON macros have also been transformed to the underlying
thread.c calls.

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>

12 files changed, 72 insertions, 98 deletions

diff --git a/ospfd/ospf_abr.c b/ospfd/ospf_abr.c
index a54170e04..ee90ceccf 100644
--- a/ospfd/ospf_abr.c
+++ b/ospfd/ospf_abr.c
@@ -1881,7 +1881,6 @@ ospf_schedule_abr_task (struct ospf *ospf)
   if (IS_DEBUG_OSPF_EVENT)
     zlog_debug ("Scheduling ABR task");
 
-  if (ospf->t_abr_task == NULL)
-    ospf->t_abr_task = thread_add_timer (master, ospf_abr_task_timer,
-					 ospf, OSPF_ABR_TASK_DELAY);
+  thread_add_timer(master, ospf_abr_task_timer, ospf, OSPF_ABR_TASK_DELAY,
+                   &ospf->t_abr_task);
 }
diff --git a/ospfd/ospf_apiserver.c b/ospfd/ospf_apiserver.c
index 620dacb15..5c514e18f 100644
--- a/ospfd/ospf_apiserver.c
+++ b/ospfd/ospf_apiserver.c
@@ -301,30 +301,32 @@ ospf_apiserver_event (enum event event, int fd,
   switch (event)
     {
     case OSPF_APISERVER_ACCEPT:
-      (void)thread_add_read (master, ospf_apiserver_accept, apiserv, fd);
+      (void) thread_add_read(master, ospf_apiserver_accept, apiserv, fd, NULL);
       break;
     case OSPF_APISERVER_SYNC_READ:
       apiserv->t_sync_read =
-	thread_add_read (master, ospf_apiserver_read, apiserv, fd);
+	thread_add_read(master, ospf_apiserver_read, apiserv, fd, NULL);
       break;
 #ifdef USE_ASYNC_READ
     case OSPF_APISERVER_ASYNC_READ:
       apiserv->t_async_read =
-	thread_add_read (master, ospf_apiserver_read, apiserv, fd);
+	thread_add_read(master, ospf_apiserver_read, apiserv, fd, NULL);
       break;
 #endif /* USE_ASYNC_READ */
     case OSPF_APISERVER_SYNC_WRITE:
       if (!apiserv->t_sync_write)
 	{
 	  apiserv->t_sync_write =
-	    thread_add_write (master, ospf_apiserver_sync_write, apiserv, fd);
+	    thread_add_write(master, ospf_apiserver_sync_write, apiserv, fd,
+                             NULL);
 	}
       break;
     case OSPF_APISERVER_ASYNC_WRITE:
       if (!apiserv->t_async_write)
 	{
 	  apiserv->t_async_write =
-	    thread_add_write (master, ospf_apiserver_async_write, apiserv, fd);
+	    thread_add_write(master, ospf_apiserver_async_write, apiserv, fd,
+                             NULL);
 	}
       break;
     }
diff --git a/ospfd/ospf_ase.c b/ospfd/ospf_ase.c
index b063f317e..a30df1993 100644
--- a/ospfd/ospf_ase.c
+++ b/ospfd/ospf_ase.c
@@ -705,9 +705,8 @@ ospf_ase_calculate_timer_add (struct ospf *ospf)
   if (ospf == NULL)
     return;
 
-  if (! ospf->t_ase_calc)
-    ospf->t_ase_calc = thread_add_timer (master, ospf_ase_calculate_timer,
-					 ospf, OSPF_ASE_CALC_INTERVAL);
+  thread_add_timer(master, ospf_ase_calculate_timer, ospf,
+                   OSPF_ASE_CALC_INTERVAL, &ospf->t_ase_calc);
 }
 
 void
diff --git a/ospfd/ospf_ism.h b/ospfd/ospf_ism.h
index fa8e6d70f..180000ae9 100644
--- a/ospfd/ospf_ism.h
+++ b/ospfd/ospf_ism.h
@@ -48,30 +48,23 @@
 #define ISM_InterfaceDown                 7
 #define OSPF_ISM_EVENT_MAX                8
 
-#define OSPF_ISM_WRITE_ON(O)                                                  \
-      do                                                                      \
-        {                                                                     \
-          if (oi->on_write_q == 0)                                            \
-	    {                                                                 \
-              listnode_add ((O)->oi_write_q, oi);                             \
-	      oi->on_write_q = 1;                                             \
-	    }                                                                 \
-	  if ((O)->t_write == NULL)                                           \
-	    (O)->t_write =                                                    \
-	      thread_add_write (master, ospf_write, (O), (O)->fd);            \
-        } while (0)
+#define OSPF_ISM_WRITE_ON(O) \
+  do \
+    { \
+      if (oi->on_write_q == 0) \
+        { \
+          listnode_add ((O)->oi_write_q, oi); \
+          oi->on_write_q = 1; \
+        } \
+      thread_add_write (master, ospf_write, (O), (O)->fd, &(O)->t_write); \
+    } while (0)
      
 /* Macro for OSPF ISM timer turn on. */
 #define OSPF_ISM_TIMER_ON(T,F,V) \
-  do { \
-    if (!(T)) \
-      (T) = thread_add_timer (master, (F), oi, (V)); \
-  } while (0)
+    thread_add_timer (master, (F), oi, (V), &(T))
+
 #define OSPF_ISM_TIMER_MSEC_ON(T,F,V) \
-  do { \
-    if (!(T)) \
-      (T) = thread_add_timer_msec (master, (F), oi, (V)); \
-  } while (0)
+    thread_add_timer_msec (master, (F), oi, (V), &(T))
 
 /* convenience macro to set hello timer correctly, according to
  * whether fast-hello is set or not
@@ -98,7 +91,7 @@
 
 /* Macro for OSPF schedule event. */
 #define OSPF_ISM_EVENT_SCHEDULE(I,E) \
-      thread_add_event (master, ospf_ism_event, (I), (E))
+      thread_add_event (master, ospf_ism_event, (I), (E), NULL)
 
 /* Macro for OSPF execute event. */
 #define OSPF_ISM_EVENT_EXECUTE(I,E) \
diff --git a/ospfd/ospf_lsa.c b/ospfd/ospf_lsa.c
index cf9943893..570e15176 100644
--- a/ospfd/ospf_lsa.c
+++ b/ospfd/ospf_lsa.c
@@ -3553,7 +3553,7 @@ ospf_schedule_lsa_flood_area (struct ospf_area *area, struct ospf_lsa *lsa)
   data->area = area;
   data->lsa  = ospf_lsa_lock (lsa); /* Message / Flood area */
 
-  thread_add_event (master, ospf_lsa_action, data, 0);
+  thread_add_event(master, ospf_lsa_action, data, 0, NULL);
 }
 
 void
@@ -3566,7 +3566,7 @@ ospf_schedule_lsa_flush_area (struct ospf_area *area, struct ospf_lsa *lsa)
   data->area = area;
   data->lsa  = ospf_lsa_lock (lsa); /* Message / Flush area */
 
-  thread_add_event (master, ospf_lsa_action, data, 0);
+  thread_add_event(master, ospf_lsa_action, data, 0, NULL);
 }
 
 
@@ -3741,8 +3741,9 @@ ospf_lsa_refresh_walker (struct thread *t)
 	}
     }
 
-  ospf->t_lsa_refresher = thread_add_timer (master, ospf_lsa_refresh_walker,
-					   ospf, ospf->lsa_refresh_interval);
+  ospf->t_lsa_refresher = thread_add_timer(master, ospf_lsa_refresh_walker,
+                                           ospf, ospf->lsa_refresh_interval,
+                                           NULL);
   ospf->lsa_refresher_started = monotime(NULL);
 
   for (ALL_LIST_ELEMENTS (lsa_to_refresh, node, nnode, lsa))
diff --git a/ospfd/ospf_nsm.h b/ospfd/ospf_nsm.h
index 4531f6ec7..2bd8f065c 100644
--- a/ospfd/ospf_nsm.h
+++ b/ospfd/ospf_nsm.h
@@ -57,11 +57,7 @@
 #define OSPF_NSM_EVENT_MAX     14
 
 /* Macro for OSPF NSM timer turn on. */
-#define OSPF_NSM_TIMER_ON(T,F,V)                                              \
-      do {                                                                    \
-        if (!(T))                                                             \
-          (T) = thread_add_timer (master, (F), nbr, (V));                     \
-      } while (0)
+#define OSPF_NSM_TIMER_ON(T,F,V) thread_add_timer (master, (F), nbr, (V), &(T))
 
 /* Macro for OSPF NSM timer turn off. */
 #define OSPF_NSM_TIMER_OFF(X)                                                 \
@@ -75,7 +71,7 @@
 
 /* Macro for OSPF NSM schedule event. */
 #define OSPF_NSM_EVENT_SCHEDULE(N,E)                                          \
-      thread_add_event (master, ospf_nsm_event, (N), (E))
+      thread_add_event (master, ospf_nsm_event, (N), (E), NULL)
 
 /* Macro for OSPF NSM execute event. */
 #define OSPF_NSM_EVENT_EXECUTE(N,E)                                           \
diff --git a/ospfd/ospf_opaque.c b/ospfd/ospf_opaque.c
index 56efa2ebd..0c1830166 100644
--- a/ospfd/ospf_opaque.c
+++ b/ospfd/ospf_opaque.c
@@ -1343,7 +1343,8 @@ ospf_opaque_lsa_originate_schedule (struct ospf_interface *oi, int *delay0)
       if (IS_DEBUG_OSPF_EVENT)
         zlog_debug ("Schedule Type-9 Opaque-LSA origination in %d ms later.", delay);
       oi->t_opaque_lsa_self =
-	thread_add_timer_msec (master, ospf_opaque_type9_lsa_originate, oi, delay);
+	thread_add_timer_msec(master, ospf_opaque_type9_lsa_originate, oi,
+                              delay, NULL);
       delay += top->min_ls_interval;
     }
 
@@ -1359,8 +1360,8 @@ ospf_opaque_lsa_originate_schedule (struct ospf_interface *oi, int *delay0)
       if (IS_DEBUG_OSPF_EVENT)
         zlog_debug ("Schedule Type-10 Opaque-LSA origination in %d ms later.", delay);
       area->t_opaque_lsa_self =
-        thread_add_timer_msec (master, ospf_opaque_type10_lsa_originate,
-			       area, delay);
+        thread_add_timer_msec(master, ospf_opaque_type10_lsa_originate, area,
+                              delay, NULL);
       delay += top->min_ls_interval;
     }
 
@@ -1376,8 +1377,8 @@ ospf_opaque_lsa_originate_schedule (struct ospf_interface *oi, int *delay0)
       if (IS_DEBUG_OSPF_EVENT)
         zlog_debug ("Schedule Type-11 Opaque-LSA origination in %d ms later.", delay);
       top->t_opaque_lsa_self =
-        thread_add_timer_msec (master, ospf_opaque_type11_lsa_originate,
-			       top, delay);
+        thread_add_timer_msec(master, ospf_opaque_type11_lsa_originate, top,
+                              delay, NULL);
       delay += top->min_ls_interval;
     }
 
@@ -1654,9 +1655,7 @@ ospf_opaque_lsa_refresh (struct ospf_lsa *lsa)
  * triggered by external interventions (vty session, signaling, etc).
  *------------------------------------------------------------------------*/
 
-#define OSPF_OPAQUE_TIMER_ON(T,F,L,V) \
-      if (!(T)) \
-        (T) = thread_add_timer_msec (master, (F), (L), (V))
+#define OSPF_OPAQUE_TIMER_ON(T,F,L,V) thread_add_timer_msec (master, (F), (L), (V), &(T))
 
 static struct ospf_lsa *pseudo_lsa (struct ospf_interface *oi, struct ospf_area *area, u_char lsa_type, u_char opaque_type);
 static int ospf_opaque_type9_lsa_reoriginate_timer (struct thread *t);
diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
index b7721adb3..6b5b0e4ed 100644
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -487,7 +487,7 @@ ospf_ls_req_event (struct ospf_neighbor *nbr)
       thread_cancel (nbr->t_ls_req);
       nbr->t_ls_req = NULL;
     }
-  nbr->t_ls_req = thread_add_event (master, ospf_ls_req_timer, nbr, 0);
+  nbr->t_ls_req = thread_add_event(master, ospf_ls_req_timer, nbr, 0, NULL);
 }
 
 /* Cyclic timer function.  Fist registered in ospf_nbr_new () in
@@ -852,7 +852,7 @@ ospf_write (struct thread *thread)
   /* If packets still remain in queue, call write thread. */
   if (!list_isempty (ospf->oi_write_q))
     ospf->t_write =                                              
-      thread_add_write (master, ospf_write, ospf, ospf->fd);
+      thread_add_write(master, ospf_write, ospf, ospf->fd, NULL);
 
   return 0;
 }
@@ -2772,7 +2772,7 @@ ospf_read (struct thread *thread)
   ospf = THREAD_ARG (thread);
 
   /* prepare for next packet. */
-  ospf->t_read = thread_add_read (master, ospf_read, ospf, ospf->fd);
+  ospf->t_read = thread_add_read(master, ospf_read, ospf, ospf->fd, NULL);
 
   stream_reset(ospf->ibuf);
   if (!(ibuf = ospf_recv_packet (ospf->fd, &ifp, ospf->ibuf)))
@@ -3803,7 +3803,7 @@ ospf_ls_upd_send_queue_event (struct thread *thread)
         zlog_debug ("ospf_ls_upd_send_queue: update lists not cleared,"
                    " %d nodes to try again, raising new event", again);
       oi->t_ls_upd_event = 
-        thread_add_event (master, ospf_ls_upd_send_queue_event, oi, 0);
+        thread_add_event(master, ospf_ls_upd_send_queue_event, oi, 0, NULL);
     }
 
   if (IS_DEBUG_OSPF_EVENT)
@@ -3858,9 +3858,8 @@ ospf_ls_upd_send (struct ospf_neighbor *nbr, struct list *update, int flag)
   for (ALL_LIST_ELEMENTS_RO (update, node, lsa))
     listnode_add (rn->info, ospf_lsa_lock (lsa)); /* oi->ls_upd_queue */
 
-  if (oi->t_ls_upd_event == NULL)
-    oi->t_ls_upd_event =
-      thread_add_event (master, ospf_ls_upd_send_queue_event, oi, 0);
+  thread_add_event(master, ospf_ls_upd_send_queue_event, oi, 0,
+                   &oi->t_ls_upd_event);
 }
 
 static void
@@ -3921,9 +3920,8 @@ ospf_ls_ack_send (struct ospf_neighbor *nbr, struct ospf_lsa *lsa)
   
   listnode_add (oi->ls_ack_direct.ls_ack, ospf_lsa_lock (lsa));
   
-  if (oi->t_ls_ack_direct == NULL)
-    oi->t_ls_ack_direct =
-      thread_add_event (master, ospf_ls_ack_send_event, oi, 0);
+  thread_add_event(master, ospf_ls_ack_send_event, oi, 0,
+                   &oi->t_ls_ack_direct);
 }
 
 /* Send Link State Acknowledgment delayed. */
diff --git a/ospfd/ospf_spf.c b/ospfd/ospf_spf.c
index 31f0d9d28..d54ebc8b4 100644
--- a/ospfd/ospf_spf.c
+++ b/ospfd/ospf_spf.c
@@ -1465,5 +1465,5 @@ ospf_spf_calculate_schedule (struct ospf *ospf, ospf_spf_reason_t reason)
   zlog_info ("SPF: Scheduled in %ld msec", delay);
 
   ospf->t_spf_calc =
-    thread_add_timer_msec (master, ospf_spf_calculate_timer, ospf, delay);
+    thread_add_timer_msec(master, ospf_spf_calculate_timer, ospf, delay, NULL);
 }
diff --git a/ospfd/ospf_zebra.c b/ospfd/ospf_zebra.c
index abb6db034..314c9e72b 100644
--- a/ospfd/ospf_zebra.c
+++ b/ospfd/ospf_zebra.c
@@ -883,7 +883,7 @@ ospf_redistribute_default_set (struct ospf *ospf, int originate,
   if (ospf->router_id.s_addr == 0)
     ospf->external_origin |= (1 << DEFAULT_ROUTE);
   else
-    thread_add_timer (master, ospf_default_originate_timer, ospf, 1);
+    thread_add_timer(master, ospf_default_originate_timer, ospf, 1, NULL);
 
   ospf_asbr_status_update (ospf, ++ospf->redistribute);
 
@@ -1279,8 +1279,8 @@ ospf_distribute_list_update (struct ospf *ospf, uintptr_t type,
 
   /* Set timer. */
   ospf->t_distribute_update =
-    thread_add_timer_msec (master, ospf_distribute_list_update_timer,
-                           (void *) type, ospf->min_ls_interval);
+    thread_add_timer_msec(master, ospf_distribute_list_update_timer,
+                          (void *)type, ospf->min_ls_interval, NULL);
 }
 
 /* If access-list is updated, apply some check. */
diff --git a/ospfd/ospfd.c b/ospfd/ospfd.c
index 30b1e9622..40f35c6d3 100644
--- a/ospfd/ospfd.c
+++ b/ospfd/ospfd.c
@@ -135,11 +135,12 @@ ospf_router_id_update (struct ospf *ospf)
 	  /* Originate each redistributed external route. */
 	  for (type = 0; type < ZEBRA_ROUTE_MAX; type++)
 	    if (ospf->external_origin & (1 << type))
-	      thread_add_event (master, ospf_external_lsa_originate_timer,
-				ospf, type);
+	      thread_add_event(master, ospf_external_lsa_originate_timer,
+                               ospf, type, NULL);
 	  /* Originate Deafult. */
 	  if (ospf->external_origin & (1 << ZEBRA_ROUTE_MAX))
-	    thread_add_event (master, ospf_default_originate_timer, ospf, 0);
+	    thread_add_event(master, ospf_default_originate_timer, ospf, 0,
+                             NULL);
 
 	  ospf->external_origin = 0;
 	}
@@ -184,9 +185,9 @@ ospf_router_id_update (struct ospf *ospf)
 
       /* Originate each redistributed external route. */
       for (type = 0; type < ZEBRA_ROUTE_MAX; type++)
-        thread_add_event (master, ospf_external_lsa_originate_timer,
-                          ospf, type);
-      thread_add_event (master, ospf_default_originate_timer, ospf, 0);
+        thread_add_event(master, ospf_external_lsa_originate_timer, ospf,
+                         type, NULL);
+      thread_add_event(master, ospf_default_originate_timer, ospf, 0, NULL);
 
       /* update router-lsa's for each area */
       ospf_router_lsa_update (ospf);
@@ -264,16 +265,17 @@ ospf_new (u_short instance)
   new->maxage_delay = OSPF_LSA_MAXAGE_REMOVE_DELAY_DEFAULT;
   new->maxage_lsa = route_table_init();
   new->t_maxage_walker =
-    thread_add_timer (master, ospf_lsa_maxage_walker,
-                      new, OSPF_LSA_MAXAGE_CHECK_INTERVAL);
+    thread_add_timer(master, ospf_lsa_maxage_walker, new,
+                     OSPF_LSA_MAXAGE_CHECK_INTERVAL, NULL);
 
   /* Distance table init. */
   new->distance_table = route_table_init ();
 
   new->lsa_refresh_queue.index = 0;
   new->lsa_refresh_interval = OSPF_LSA_REFRESH_INTERVAL_DEFAULT;
-  new->t_lsa_refresher = thread_add_timer (master, ospf_lsa_refresh_walker,
-					   new, new->lsa_refresh_interval);
+  new->t_lsa_refresher = thread_add_timer(master, ospf_lsa_refresh_walker,
+                                          new, new->lsa_refresh_interval,
+                                          NULL);
   new->lsa_refresher_started = monotime(NULL);
 
   if ((new->fd = ospf_sock_init()) < 0)
@@ -288,7 +290,7 @@ ospf_new (u_short instance)
 	       OSPF_MAX_PACKET_SIZE+1);
       exit(1);
     }
-  new->t_read = thread_add_read (master, ospf_read, new, new->fd);
+  new->t_read = thread_add_read(master, ospf_read, new, new->fd, NULL);
   new->oi_write_q = list_new ();
   new->write_oi_count = OSPF_WRITE_INTERFACE_COUNT_DEFAULT;
   
@@ -1593,7 +1595,8 @@ ospf_timers_refresh_set (struct ospf *ospf, int interval)
     {
       OSPF_TIMER_OFF (ospf->t_lsa_refresher);
       ospf->t_lsa_refresher =
-	thread_add_timer (master, ospf_lsa_refresh_walker, ospf, interval);
+	thread_add_timer(master, ospf_lsa_refresh_walker, ospf, interval,
+                         NULL);
     }
   ospf->lsa_refresh_interval = interval;
 
@@ -1612,8 +1615,8 @@ ospf_timers_refresh_unset (struct ospf *ospf)
     {
       OSPF_TIMER_OFF (ospf->t_lsa_refresher);
       ospf->t_lsa_refresher =
-	thread_add_timer (master, ospf_lsa_refresh_walker, ospf,
-			  OSPF_LSA_REFRESH_INTERVAL_DEFAULT);
+	thread_add_timer(master, ospf_lsa_refresh_walker, ospf,
+                         OSPF_LSA_REFRESH_INTERVAL_DEFAULT, NULL);
     }
 
   ospf->lsa_refresh_interval = OSPF_LSA_REFRESH_INTERVAL_DEFAULT;
diff --git a/ospfd/ospfd.h b/ospfd/ospfd.h
index 9198d5c62..c5b0324b8 100644
--- a/ospfd/ospfd.h
+++ b/ospfd/ospfd.h
@@ -480,26 +480,10 @@ struct ospf_nbr_nbma
 #define LSA_OPTIONS_NSSA_GET(area) \
         (((area)->external_routing == OSPF_AREA_NSSA)  ? OSPF_OPTION_NP : 0)
 
-#define OSPF_TIMER_ON(T,F,V)                                                  \
-    do {                                                                      \
-      if (!(T))                                                               \
-	(T) = thread_add_timer (master, (F), ospf, (V));                      \
-    } while (0)
-
-#define OSPF_AREA_TIMER_ON(T,F,V)                                             \
-    do {                                                                      \
-      if (!(T))                                                               \
-        (T) = thread_add_timer (master, (F), area, (V));                      \
-    } while (0)
-
-#define OSPF_POLL_TIMER_ON(T,F,V)                                             \
-    do {                                                                      \
-      if (!(T))                                                               \
-        (T) = thread_add_timer (master, (F), nbr_nbma, (V));                  \
-    } while (0)
-
-#define OSPF_POLL_TIMER_OFF(X)		OSPF_TIMER_OFF((X))
-
+#define OSPF_TIMER_ON(T,F,V) thread_add_timer (master,(F),ospf,(V),&(T))
+#define OSPF_AREA_TIMER_ON(T,F,V) thread_add_timer (master, (F), area, (V), &(T))
+#define OSPF_POLL_TIMER_ON(T,F,V) thread_add_timer (master, (F), nbr_nbma, (V), &(T))
+#define OSPF_POLL_TIMER_OFF(X) OSPF_TIMER_OFF((X))
 #define OSPF_TIMER_OFF(X)                                                     \
     do {                                                                      \
       if (X)                                                                  \