]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Refactor isc_timer_create() to just create timer
authorOndřej Surý <ondrej@isc.org>
Fri, 11 Mar 2022 11:09:35 +0000 (12:09 +0100)
committerEvan Hunt <each@isc.org>
Mon, 14 Mar 2022 20:00:05 +0000 (13:00 -0700)
The isc_timer_create() function was a bit conflated.  It could have been
used to create a timer and start it at the same time.  As there was a
single place where this was done before (see the previous commit for
nta.c), this was cleaned up and the isc_timer_create() function was
changed to only create new timer.

12 files changed:
bin/named/server.c
doc/dev/dev.md
lib/dns/catz.c
lib/dns/nta.c
lib/dns/resolver.c
lib/dns/rpz.c
lib/dns/zone.c
lib/isc/include/isc/timer.h
lib/isc/ratelimiter.c
lib/isc/tests/task_test.c
lib/isc/tests/timer_test.c
lib/isc/timer.c

index 2c5cc4253fa19de30ad0cb245effd55a2f983c25..7bb62cb1e0fc9aed144eb9b916b235475c0cc095 100644 (file)
@@ -9942,27 +9942,17 @@ run_server(isc_task_t *task, isc_event_t *event) {
                                          true, &server->interfacemgr),
                   "creating interface manager");
 
-       CHECKFATAL(isc_timer_create(named_g_timermgr, isc_timertype_inactive,
-                                   NULL, NULL, server->task,
-                                   interface_timer_tick, server,
-                                   &server->interface_timer),
-                  "creating interface timer");
-
-       CHECKFATAL(isc_timer_create(named_g_timermgr, isc_timertype_inactive,
-                                   NULL, NULL, server->task,
-                                   heartbeat_timer_tick, server,
-                                   &server->heartbeat_timer),
-                  "creating heartbeat timer");
-
-       CHECKFATAL(isc_timer_create(named_g_timermgr, isc_timertype_inactive,
-                                   NULL, NULL, server->task, tat_timer_tick,
-                                   server, &server->tat_timer),
-                  "creating trust anchor telemetry timer");
-
-       CHECKFATAL(isc_timer_create(named_g_timermgr, isc_timertype_inactive,
-                                   NULL, NULL, server->task, pps_timer_tick,
-                                   server, &server->pps_timer),
-                  "creating pps timer");
+       isc_timer_create(named_g_timermgr, server->task, interface_timer_tick,
+                        server, &server->interface_timer);
+
+       isc_timer_create(named_g_timermgr, server->task, heartbeat_timer_tick,
+                        server, &server->heartbeat_timer);
+
+       isc_timer_create(named_g_timermgr, server->task, tat_timer_tick, server,
+                        &server->tat_timer);
+
+       isc_timer_create(named_g_timermgr, server->task, pps_timer_tick, server,
+                        &server->pps_timer);
 
        CHECKFATAL(
                cfg_parser_create(named_g_mctx, named_g_lctx, &named_g_parser),
index bf7aa94020e10629a9d1e214411731d174d2d914..be26ba6a33d32425b2d18cef1d492703edf30a33 100644 (file)
@@ -1400,8 +1400,9 @@ be triggered at that time.
           * 'arg' as its argument in task 'task'.
           */
          isc_timer_t *timer = NULL;
-         result = isc_timer_create(timermgr, isc_timertype_once, NULL,
-                                   interval, task, timeout, arg, &timer);
+         result = isc_timer_create(timermgr, task, timeout, arg, &timer);
+         result = isc_timer_reset(timermgr, isc_timertype_once, NULL,
+                                  interval, false);
 
 An event can also be explicitly triggered via `isc_task_send()`.  
 
index 393d8454a5df7082b6a4104ca5656cc964d8f0bc..7459f7f6866446b263416322022f34cbaef04085 100644 (file)
@@ -617,7 +617,6 @@ dns_catz_catzs_set_view(dns_catz_zones_t *catzs, dns_view_t *view) {
 isc_result_t
 dns_catz_new_zone(dns_catz_zones_t *catzs, dns_catz_zone_t **zonep,
                  const dns_name_t *name) {
-       isc_result_t result;
        dns_catz_zone_t *new_zone;
 
        REQUIRE(DNS_CATZ_ZONES_VALID(catzs));
@@ -634,13 +633,9 @@ dns_catz_new_zone(dns_catz_zones_t *catzs, dns_catz_zone_t **zonep,
        isc_ht_init(&new_zone->entries, catzs->mctx, 4);
 
        new_zone->updatetimer = NULL;
-       result = isc_timer_create(catzs->timermgr, isc_timertype_inactive, NULL,
-                                 NULL, catzs->updater,
-                                 dns_catz_update_taskaction, new_zone,
-                                 &new_zone->updatetimer);
-       if (result != ISC_R_SUCCESS) {
-               goto cleanup_ht;
-       }
+       isc_timer_create(catzs->timermgr, catzs->updater,
+                        dns_catz_update_taskaction, new_zone,
+                        &new_zone->updatetimer);
 
        isc_time_settoepoch(&new_zone->lastupdated);
        new_zone->updatepending = false;
@@ -658,13 +653,6 @@ dns_catz_new_zone(dns_catz_zones_t *catzs, dns_catz_zone_t **zonep,
        *zonep = new_zone;
 
        return (ISC_R_SUCCESS);
-
-cleanup_ht:
-       isc_ht_destroy(&new_zone->entries);
-       dns_name_free(&new_zone->name, catzs->mctx);
-       isc_mem_put(catzs->mctx, new_zone, sizeof(*new_zone));
-
-       return (result);
 }
 
 isc_result_t
index a7ea202ed407dd7c5c5be58d88b80841bf52c556..1126202d237cb237e48da7898ab95d8ced746276 100644 (file)
@@ -289,13 +289,8 @@ settimer(dns_ntatable_t *ntatable, dns_nta_t *nta, uint32_t lifetime) {
                return (ISC_R_SUCCESS);
        }
 
-       result = isc_timer_create(ntatable->timermgr, isc_timertype_inactive,
-                                 NULL, NULL, ntatable->task, checkbogus, nta,
-                                 &nta->timer);
-       if (result != ISC_R_SUCCESS) {
-               return (result);
-       }
-
+       isc_timer_create(ntatable->timermgr, ntatable->task, checkbogus, nta,
+                        &nta->timer);
        isc_interval_set(&interval, view->nta_recheck, 0);
        result = isc_timer_reset(nta->timer, isc_timertype_ticker, NULL,
                                 &interval, false);
index 2c5c50ab3841408b855dc0cd96992372e106e52a..a81e4285eff50c2a46de7c144c275d77537dffef 100644 (file)
@@ -4857,15 +4857,8 @@ fctx_create(dns_resolver_t *res, isc_task_t *task, const dns_name_t *name,
         * lifetime. It will be made active when the fetch is
         * started.
         */
-       iresult = isc_timer_create(res->timermgr, isc_timertype_inactive, NULL,
-                                  NULL, res->buckets[bucketnum].task,
-                                  fctx_expired, fctx, &fctx->timer);
-       if (iresult != ISC_R_SUCCESS) {
-               UNEXPECTED_ERROR(__FILE__, __LINE__, "isc_timer_create: %s",
-                                isc_result_totext(iresult));
-               result = ISC_R_UNEXPECTED;
-               goto cleanup_qmessage;
-       }
+       isc_timer_create(res->timermgr, res->buckets[bucketnum].task,
+                        fctx_expired, fctx, &fctx->timer);
 
        /*
         * Default retry interval initialization.  We set the interval
@@ -10167,13 +10160,9 @@ dns_resolver_create(dns_view_t *view, isc_taskmgr_t *taskmgr,
        }
        isc_task_setname(task, "resolver_task", NULL);
 
-       result = isc_timer_create(timermgr, isc_timertype_inactive, NULL, NULL,
-                                 task, spillattimer_countdown, res,
-                                 &res->spillattimer);
+       isc_timer_create(timermgr, task, spillattimer_countdown, res,
+                        &res->spillattimer);
        isc_task_detach(&task);
-       if (result != ISC_R_SUCCESS) {
-               goto cleanup_primelock;
-       }
 
        res->magic = RES_MAGIC;
 
index 3b54ca6ca3c3ae41eebdd1369d2136935c581567..863345999cc5a2f5585b4053925ff5081e731188 100644 (file)
@@ -1514,7 +1514,6 @@ cleanup_rbt:
 isc_result_t
 dns_rpz_new_zone(dns_rpz_zones_t *rpzs, dns_rpz_zone_t **rpzp) {
        dns_rpz_zone_t *zone;
-       isc_result_t result;
 
        REQUIRE(rpzp != NULL && *rpzp == NULL);
        REQUIRE(rpzs != NULL);
@@ -1527,13 +1526,8 @@ dns_rpz_new_zone(dns_rpz_zones_t *rpzs, dns_rpz_zone_t **rpzp) {
        memset(zone, 0, sizeof(*zone));
        isc_refcount_init(&zone->refs, 1);
 
-       result = isc_timer_create(rpzs->timermgr, isc_timertype_inactive, NULL,
-                                 NULL, rpzs->updater,
-                                 dns_rpz_update_taskaction, zone,
-                                 &zone->updatetimer);
-       if (result != ISC_R_SUCCESS) {
-               goto cleanup_timer;
-       }
+       isc_timer_create(rpzs->timermgr, rpzs->updater,
+                        dns_rpz_update_taskaction, zone, &zone->updatetimer);
 
        /*
         * This will never be used, but costs us nothing and
@@ -1573,14 +1567,6 @@ dns_rpz_new_zone(dns_rpz_zones_t *rpzs, dns_rpz_zone_t **rpzp) {
        *rpzp = zone;
 
        return (ISC_R_SUCCESS);
-
-cleanup_timer:
-       isc_refcount_decrementz(&zone->refs);
-       isc_refcount_destroy(&zone->refs);
-
-       isc_mem_put(rpzs->mctx, zone, sizeof(*zone));
-
-       return (result);
 }
 
 isc_result_t
index 7f9f8edcba5f0cb97396539675c27ba42070e001..304bfb5a32a74b1f9c306448ddf4c81fac3db992 100644 (file)
@@ -18946,8 +18946,6 @@ dns_zonemgr_createzone(dns_zonemgr_t *zmgr, dns_zone_t **zonep) {
 
 isc_result_t
 dns_zonemgr_managezone(dns_zonemgr_t *zmgr, dns_zone_t *zone) {
-       isc_result_t result;
-
        REQUIRE(DNS_ZONE_VALID(zone));
        REQUIRE(DNS_ZONEMGR_VALID(zmgr));
 
@@ -18972,13 +18970,8 @@ dns_zonemgr_managezone(dns_zonemgr_t *zmgr, dns_zone_t *zone) {
        isc_task_setname(zone->task, "zone", zone);
        isc_task_setname(zone->loadtask, "loadzone", zone);
 
-       result = isc_timer_create(zmgr->timermgr, isc_timertype_inactive, NULL,
-                                 NULL, zone->task, zone_timer, zone,
-                                 &zone->timer);
-
-       if (result != ISC_R_SUCCESS) {
-               goto cleanup_tasks;
-       }
+       isc_timer_create(zmgr->timermgr, zone->task, zone_timer, zone,
+                        &zone->timer);
 
        /*
         * The timer "holds" a iref.
@@ -18991,16 +18984,9 @@ dns_zonemgr_managezone(dns_zonemgr_t *zmgr, dns_zone_t *zone) {
        zone->zmgr = zmgr;
        isc_refcount_increment(&zmgr->refs);
 
-       goto unlock;
-
-cleanup_tasks:
-       isc_task_detach(&zone->loadtask);
-       isc_task_detach(&zone->task);
-
-unlock:
        UNLOCK_ZONE(zone);
        RWUNLOCK(&zmgr->rwlock, isc_rwlocktype_write);
-       return (result);
+       return (ISC_R_SUCCESS);
 }
 
 void
@@ -22485,7 +22471,6 @@ dns_zone_getserialupdatemethod(dns_zone_t *zone) {
  */
 isc_result_t
 dns_zone_link(dns_zone_t *zone, dns_zone_t *raw) {
-       isc_result_t result;
        dns_zonemgr_t *zmgr;
 
        REQUIRE(DNS_ZONE_VALID(zone));
@@ -22510,12 +22495,8 @@ dns_zone_link(dns_zone_t *zone, dns_zone_t *raw) {
        LOCK_ZONE(zone);
        LOCK_ZONE(raw);
 
-       result = isc_timer_create(zmgr->timermgr, isc_timertype_inactive, NULL,
-                                 NULL, zone->task, zone_timer, raw,
-                                 &raw->timer);
-       if (result != ISC_R_SUCCESS) {
-               goto unlock;
-       }
+       isc_timer_create(zmgr->timermgr, zone->task, zone_timer, raw,
+                        &raw->timer);
 
        /*
         * The timer "holds" a iref.
@@ -22536,11 +22517,10 @@ dns_zone_link(dns_zone_t *zone, dns_zone_t *raw) {
        raw->zmgr = zmgr;
        isc_refcount_increment(&zmgr->refs);
 
-unlock:
        UNLOCK_ZONE(raw);
        UNLOCK_ZONE(zone);
        RWUNLOCK(&zmgr->rwlock, isc_rwlocktype_write);
-       return (result);
+       return (ISC_R_SUCCESS);
 }
 
 void
index 3983c4e386a7533dcd50ead4f2c8db41ef4d7b75..66c161fdb317893f2885084266b66fe12307b7e7 100644 (file)
@@ -101,11 +101,9 @@ typedef struct isc_timerevent {
  *** those functions which return an isc_result_t.
  ***/
 
-isc_result_t
-isc_timer_create(isc_timermgr_t *manager, isc_timertype_t type,
-                const isc_time_t *expires, const isc_interval_t *interval,
-                isc_task_t *task, isc_taskaction_t action, void *arg,
-                isc_timer_t **timerp);
+void
+isc_timer_create(isc_timermgr_t *manager, isc_task_t *task,
+                isc_taskaction_t action, void *arg, isc_timer_t **timerp);
 /*%<
  * Create a new 'type' timer managed by 'manager'.  The timers parameters
  * are specified by 'expires' and 'interval'.  Events will be posted to
index dabbb08ff4d7ed4bd54678c5e913b80cd6c3f280..9f48ab540110bc705017d477896c16f2d62b47ca 100644 (file)
@@ -56,7 +56,6 @@ ratelimiter_shutdowncomplete(isc_task_t *task, isc_event_t *event);
 isc_result_t
 isc_ratelimiter_create(isc_mem_t *mctx, isc_timermgr_t *timermgr,
                       isc_task_t *task, isc_ratelimiter_t **ratelimiterp) {
-       isc_result_t result;
        isc_ratelimiter_t *rl;
        INSIST(ratelimiterp != NULL && *ratelimiterp == NULL);
 
@@ -74,11 +73,7 @@ isc_ratelimiter_create(isc_mem_t *mctx, isc_timermgr_t *timermgr,
 
        isc_mutex_init(&rl->lock);
 
-       result = isc_timer_create(timermgr, isc_timertype_inactive, NULL, NULL,
-                                 rl->task, ratelimiter_tick, rl, &rl->timer);
-       if (result != ISC_R_SUCCESS) {
-               goto free_mutex;
-       }
+       isc_timer_create(timermgr, rl->task, ratelimiter_tick, rl, &rl->timer);
 
        /*
         * Increment the reference count to indicate that we may
@@ -92,13 +87,6 @@ isc_ratelimiter_create(isc_mem_t *mctx, isc_timermgr_t *timermgr,
 
        *ratelimiterp = rl;
        return (ISC_R_SUCCESS);
-
-free_mutex:
-       isc_refcount_decrementz(&rl->references);
-       isc_refcount_destroy(&rl->references);
-       isc_mutex_destroy(&rl->lock);
-       isc_mem_put(mctx, rl, sizeof(*rl));
-       return (result);
 }
 
 isc_result_t
index f2cb18739c3d1b344a73cddb5c4c8eedee3d849a..1730862d85539ec64844ae4ffd62b9a8cda2bd45 100644 (file)
@@ -508,15 +508,17 @@ basic(void **state) {
 
        isc_time_settoepoch(&absolute);
        isc_interval_set(&interval, 1, 0);
-       result = isc_timer_create(timermgr, isc_timertype_ticker, &absolute,
-                                 &interval, task1, basic_tick, tick, &ti1);
+       isc_timer_create(timermgr, task1, basic_tick, tick, &ti1);
+       result = isc_timer_reset(ti1, isc_timertype_ticker, &absolute,
+                                &interval, false);
        assert_int_equal(result, ISC_R_SUCCESS);
 
        ti2 = NULL;
        isc_time_settoepoch(&absolute);
        isc_interval_set(&interval, 1, 0);
-       result = isc_timer_create(timermgr, isc_timertype_ticker, &absolute,
-                                 &interval, task2, basic_tick, tock, &ti2);
+       isc_timer_create(timermgr, task2, basic_tick, tock, &ti2);
+       result = isc_timer_reset(ti2, isc_timertype_ticker, &absolute,
+                                &interval, false);
        assert_int_equal(result, ISC_R_SUCCESS);
 
        sleep(2);
index 8ff64c6da3b6287ce86cc0583f30ce13995df78c..ac647e0b74633542c68cb98f84c9abd1389523bc 100644 (file)
@@ -130,8 +130,8 @@ setup_test(isc_timertype_t timertype, isc_time_t *expires,
        isc_mutex_unlock(&lasttime_mx);
        assert_int_equal(result, ISC_R_SUCCESS);
 
-       result = isc_timer_create(timermgr, timertype, expires, interval, task,
-                                 action, (void *)timertype, &timer);
+       isc_timer_create(timermgr, task, action, (void *)timertype, &timer);
+       result = isc_timer_reset(timer, timertype, expires, interval, false);
        assert_int_equal(result, ISC_R_SUCCESS);
 
        /*
@@ -561,9 +561,9 @@ purge(void **state) {
        isc_interval_set(&interval, seconds, 0);
 
        tickertimer = NULL;
-       result = isc_timer_create(timermgr, isc_timertype_ticker, &expires,
-                                 &interval, task1, tick_event, NULL,
-                                 &tickertimer);
+       isc_timer_create(timermgr, task1, tick_event, NULL, &tickertimer);
+       result = isc_timer_reset(tickertimer, isc_timertype_ticker, &expires,
+                                &interval, false);
        assert_int_equal(result, ISC_R_SUCCESS);
 
        oncetimer = NULL;
@@ -573,9 +573,9 @@ purge(void **state) {
        assert_int_equal(result, ISC_R_SUCCESS);
 
        isc_interval_set(&interval, 0, 0);
-       result = isc_timer_create(timermgr, isc_timertype_once, &expires,
-                                 &interval, task2, once_event, NULL,
-                                 &oncetimer);
+       isc_timer_create(timermgr, task2, once_event, NULL, &oncetimer);
+       result = isc_timer_reset(oncetimer, isc_timertype_once, &expires,
+                                &interval, false);
        assert_int_equal(result, ISC_R_SUCCESS);
 
        /*
index 544e79655eb245e901f7c89c2860d43d408f9f11..17322b2e444f0c0429162a614c190ddfb2bfd5f6 100644 (file)
@@ -219,115 +219,54 @@ destroy(isc_timer_t *timer) {
        isc_mem_put(manager->mctx, timer, sizeof(*timer));
 }
 
-isc_result_t
-isc_timer_create(isc_timermgr_t *manager, isc_timertype_t type,
-                const isc_time_t *expires, const isc_interval_t *interval,
-                isc_task_t *task, isc_taskaction_t action, void *arg,
-                isc_timer_t **timerp) {
+void
+isc_timer_create(isc_timermgr_t *manager, isc_task_t *task,
+                isc_taskaction_t action, void *arg, isc_timer_t **timerp) {
        REQUIRE(VALID_MANAGER(manager));
        REQUIRE(task != NULL);
        REQUIRE(action != NULL);
 
        isc_timer_t *timer;
-       isc_result_t result;
        isc_time_t now;
 
-       /*
-        * Create a new 'type' timer managed by 'manager'.  The timers
-        * parameters are specified by 'expires' and 'interval'.  Events
-        * will be posted to 'task' and when dispatched 'action' will be
-        * called with 'arg' as the arg value.  The new timer is returned
-        * in 'timerp'.
-        */
-       if (expires == NULL) {
-               expires = isc_time_epoch;
-       }
-       if (interval == NULL) {
-               interval = isc_interval_zero;
-       }
-       REQUIRE(type == isc_timertype_inactive ||
-               !(isc_time_isepoch(expires) && isc_interval_iszero(interval)));
        REQUIRE(timerp != NULL && *timerp == NULL);
 
        /*
         * Get current time.
         */
-       if (type != isc_timertype_inactive) {
-               TIME_NOW(&now);
-       } else {
-               /*
-                * We don't have to do this, but it keeps the compiler from
-                * complaining about "now" possibly being used without being
-                * set, even though it will never actually happen.
-                */
-               isc_time_settoepoch(&now);
-       }
+       TIME_NOW(&now);
 
        timer = isc_mem_get(manager->mctx, sizeof(*timer));
+       *timer = (isc_timer_t){
+               .manager = manager,
+               .type = isc_timertype_inactive,
+               .expires = *isc_time_epoch,
+               .interval = *isc_interval_zero,
+               .action = action,
+               .arg = arg,
+       };
 
-       timer->manager = manager;
        isc_refcount_init(&timer->references, 1);
 
-       if (type == isc_timertype_once && !isc_interval_iszero(interval)) {
-               result = isc_time_add(&now, interval, &timer->idle);
-               if (result != ISC_R_SUCCESS) {
-                       isc_mem_put(manager->mctx, timer, sizeof(*timer));
-                       return (result);
-               }
-       } else {
-               isc_time_settoepoch(&timer->idle);
-       }
+       isc_time_settoepoch(&timer->idle);
 
-       timer->type = type;
-       timer->expires = *expires;
-       timer->interval = *interval;
-       timer->task = NULL;
        isc_task_attach(task, &timer->task);
-       timer->action = action;
-       /*
-        * Removing the const attribute from "arg" is the best of two
-        * evils here.  If the timer->arg member is made const, then
-        * it affects a great many recipients of the timer event
-        * which did not pass in an "arg" that was truly const.
-        * Changing isc_timer_create() to not have "arg" prototyped as const,
-        * though, can cause compilers warnings for calls that *do*
-        * have a truly const arg.  The caller will have to carefully
-        * keep track of whether arg started as a true const.
-        */
-       DE_CONST(arg, timer->arg);
-       timer->index = 0;
+
        isc_mutex_init(&timer->lock);
        ISC_LINK_INIT(timer, link);
-       timer->magic = TIMER_MAGIC;
 
-       LOCK(&manager->lock);
+       timer->magic = TIMER_MAGIC;
 
        /*
         * Note we don't have to lock the timer like we normally would because
         * there are no external references to it yet.
         */
 
-       if (type != isc_timertype_inactive) {
-               result = schedule(timer, &now, true);
-       } else {
-               result = ISC_R_SUCCESS;
-       }
-       if (result == ISC_R_SUCCESS) {
-               *timerp = timer;
-               APPEND(manager->timers, timer, link);
-       }
+       *timerp = timer;
 
+       LOCK(&manager->lock);
+       APPEND(manager->timers, timer, link);
        UNLOCK(&manager->lock);
-
-       if (result != ISC_R_SUCCESS) {
-               timer->magic = 0;
-               isc_mutex_destroy(&timer->lock);
-               isc_task_detach(&timer->task);
-               isc_mem_put(manager->mctx, timer, sizeof(*timer));
-               return (result);
-       }
-
-       return (ISC_R_SUCCESS);
 }
 
 isc_result_t