Re: [PATCH 1/3] cyclic: make cyclic_unregister() idempotent

[Stefan Roese <sr@denx.de>]

On 07.05.25 12:58, Rasmus Villemoes wrote:
> Make cyclic_unregister() safe to call with an already unregistered, or
> possibly never registered, struct cyclic_info. This is similar to how
> the various timer APIs in the linux kernel work (they all allow
> calling delete/cancel/... on an inactive timer object).
> 
> This means callers don't have to separately keep track of whether
> their cyclic callback is registered or not, and avoids them trying to
> peek into the struct cyclic_info for that information - which leads to
> somewhat ugly code as it would have to be guarded by ifdef
> CONFIG_CYCLIC.
> 
> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   common/cyclic.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/common/cyclic.c b/common/cyclic.c
> index b695f092f52..75662d9f613 100644
> --- a/common/cyclic.c
> +++ b/common/cyclic.c
> @@ -28,6 +28,18 @@ struct hlist_head *cyclic_get_list(void)
>   	return (struct hlist_head *)&gd->cyclic_list;
>   }
>   
> +static bool cyclic_is_registered(const struct cyclic_info *cyclic)
> +{
> +	const struct cyclic_info *c;
> +
> +	hlist_for_each_entry(c, cyclic_get_list(), list) {
> +		if (c == cyclic)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
>   void cyclic_register(struct cyclic_info *cyclic, cyclic_func_t func,
>   		     uint64_t delay_us, const char *name)
>   {
> @@ -43,6 +55,9 @@ void cyclic_register(struct cyclic_info *cyclic, cyclic_func_t func,
>   
>   void cyclic_unregister(struct cyclic_info *cyclic)
>   {
> +	if (!cyclic_is_registered(cyclic))
> +		return;
> +
>   	hlist_del(&cyclic->list);
>   }
>   

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de