From: Florian Obser <florian@openbsd.org>
Subject: Re: [PATCH 4/4] evtimer_del() and signal_del(): remove redundant pending checks in daemons
To: Bjorn Ketelaars <bket@openbsd.org>
Cc: tech@openbsd.org
Date: Wed, 21 Oct 2026 21:22:33 +0200

reads OK florian

On 2026-10-16 14:35 +02, Bjorn Ketelaars <bket@openbsd.org> wrote:
> event(3) says event_del() has no effect if an event “has already
> executed or has never been added”. It describes evtimer_del() and
> signal_del() as abbreviations for the same event API, so checking
> whether a timer or signal event is pending immediately before deleting
> it is unnecessary.
>
> This patch removes those checks from cleanup paths in ldapd, radiusd,
> and vmd.
>
> These are mechanical cleanups; event scheduling and callback behavior
> are unchanged.
>
> Comments/OK?
>
> Index: usr.sbin/ldapd/namespace.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldapd/namespace.c,v
> diff -u -p -r1.20 namespace.c
> --- usr.sbin/ldapd/namespace.c	5 Mar 2020 07:39:25 -0000	1.20
> +++ usr.sbin/ldapd/namespace.c	16 Oct 2026 12:25:44 -0000
> @@ -238,8 +238,7 @@ namespace_close(struct namespace *ns)
>  	free(ns->suffix);
>  	btree_close(ns->data_db);
>  	btree_close(ns->indx_db);
> -	if (evtimer_pending(&ns->ev_queue, NULL))
> -		evtimer_del(&ns->ev_queue);
> +	evtimer_del(&ns->ev_queue);
>  	free(ns->data_path);
>  	free(ns->indx_path);
>  	free(ns);
> Index: usr.sbin/radiusd/radiusd.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/radiusd/radiusd.c,v
> diff -u -p -r1.64 radiusd.c
> --- usr.sbin/radiusd/radiusd.c	4 Aug 2026 13:24:44 -0000	1.64
> +++ usr.sbin/radiusd/radiusd.c	16 Oct 2026 12:25:44 -0000
> @@ -323,14 +323,10 @@ radiusd_stop(struct radiusd *radiusd)
>  		radiusd_module_stop(module);
>  		radiusd_module_close(module);
>  	}
> -	if (signal_pending(&radiusd->ev_sigterm, NULL))
> -		signal_del(&radiusd->ev_sigterm);
> -	if (signal_pending(&radiusd->ev_sigint, NULL))
> -		signal_del(&radiusd->ev_sigint);
> -	if (signal_pending(&radiusd->ev_sighup, NULL))
> -		signal_del(&radiusd->ev_sighup);
> -	if (signal_pending(&radiusd->ev_sigchld, NULL))
> -		signal_del(&radiusd->ev_sigchld);
> +	signal_del(&radiusd->ev_sigterm);
> +	signal_del(&radiusd->ev_sigint);
> +	signal_del(&radiusd->ev_sighup);
> +	signal_del(&radiusd->ev_sigchld);
>  }
>  
>  static void
> Index: usr.sbin/radiusd/radiusd_ipcp.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/radiusd/radiusd_ipcp.c,v
> diff -u -p -r1.27 radiusd_ipcp.c
> --- usr.sbin/radiusd/radiusd_ipcp.c	25 Jun 2025 11:38:21 -0000	1.27
> +++ usr.sbin/radiusd/radiusd_ipcp.c	16 Oct 2026 12:25:44 -0000
> @@ -349,8 +349,7 @@ ipcp_stop(void *ctx)
>  		if (evtimer_pending(&dae->ev_reqs, NULL))
>  			event_del(&dae->ev_reqs);
>  	}
> -	if (evtimer_pending(&self->ev_timer, NULL))
> -		evtimer_del(&self->ev_timer);
> +	evtimer_del(&self->ev_timer);
>  }
>  
>  void
> @@ -379,8 +378,7 @@ ipcp_fini(struct module_ipcp *self)
>  	}
>  	TAILQ_FOREACH_SAFE(addr, &self->addrs, next, addrt)
>  		free(addr);
> -	if (evtimer_pending(&self->ev_timer, NULL))
> -		evtimer_del(&self->ev_timer);
> +	evtimer_del(&self->ev_timer);
>  	module_destroy(self->base);
>  }
>  
> @@ -1640,8 +1638,7 @@ ipcp_schedule_timer(struct module_ipcp *
>  		    timespeccmp(&min_assign->timeout, &assign->timeout, >))
>  			min_assign = assign;
>  	}
> -	if (evtimer_pending(&self->ev_timer, NULL))
> -		evtimer_del(&self->ev_timer);
> +	evtimer_del(&self->ev_timer);
>  
>  	if (min_assign != NULL) {
>  		timespecsub(&min_assign->timeout, &self->uptime, &tsd);
> @@ -1846,8 +1843,7 @@ ipcp_dae_reset_request(struct assigned_i
>  	if (assign->dae_reqpkt != NULL)
>  		radius_delete_packet(assign->dae_reqpkt);
>  	assign->dae_reqpkt = NULL;
> -	if (evtimer_pending(&assign->dae_evtimer, NULL))
> -		evtimer_del(&assign->dae_evtimer);
> +	evtimer_del(&assign->dae_evtimer);
>  	TAILQ_FOREACH_SAFE(client, &assign->dae_clients, entry, clientt) {
>  		TAILQ_REMOVE(&assign->dae_clients, client, entry);
>  		free(client);
> Index: usr.sbin/vmd/mc146818.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/vmd/mc146818.c,v
> diff -u -p -r1.34 mc146818.c
> --- usr.sbin/vmd/mc146818.c	19 Sep 2026 17:21:52 -0000	1.34
> +++ usr.sbin/vmd/mc146818.c	16 Oct 2026 12:25:44 -0000
> @@ -214,8 +214,7 @@ rtc_reschedule_per(void)
>  	uint64_t us;
>  	uint8_t period;
>  
> -	if (evtimer_pending(&rtc.per, NULL))
> -		evtimer_del(&rtc.per);
> +	evtimer_del(&rtc.per);
>  
>  	if (rtc.regs[MC_REGB] & MC_REGB_PIE) {
>  		period = rtc.regs[MC_REGA] & MC_RATE_MASK;
>

-- 
In my defence, I have been left unsupervised.