Re: [PATCH] of: Build warn for missing fn() in _OF_DECLARE

[Robin Murphy <robin.murphy@arm.com>]

On 2025-04-17 2:23 pm, Liya Huang wrote:
> The function pointer fn() in _OF_DECLARE macro might be NULL. For example,
> in __reserved_mem_init_node(), only non-NULL cases are handled, and NULL
> function pointers are ignored.
> 
> This patch introduces a check to handle cases where fn() is NULL. If fn()
> is found to be NULL, a warning is issued during compilation to notify
> developers about the missing function pointer.
> 
> ---
> The function pointer fn() in _OF_DECLARE macro might be NULL. For example,
> in __reserved_mem_init_node(), only non-NULL cases are handled, and NULL
> function pointers are ignored.
> 
> This patch introduces a check to handle cases where fn() is NULL. If fn()
> is found to be NULL, a warning is issued during compilation to notify
> developers about the missing function pointer.

This patch in -next appears to be responsible for syzbot complaining 
about build errors for some configs:

"
kernel/dma/coherent.c:410:1: error: static assertion expression is not 
an integral constant expression
kernel/dma/contiguous.c:497:1: error: static assertion expression is not 
an integral constant expression
"

https://lore.kernel.org/linux-iommu/6808d00a.050a0220.7184a.0010.GAE@google.com/

Also on closer inspection, just outside the diff context we still seem 
to be explicitly anticipating fn being NULL with:

	.data = (fn == (fn_type)NULL) ? fn : fn

so something doesn't seem quite right...

Thanks,
Robin.

> Link: https://lore.kernel.org/all/CAL_JsqK733Q9bbxC0Wz5uxyZ9m7bs+bci5kUJF9GJMv73-dO4w@mail.gmail.com/
> 
> Signed-off-by: Liya Huang <1425075683@qq.com>
> ---
>   include/linux/of.h | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/include/linux/of.h b/include/linux/of.h
> index a62154aeda1b6a600c2b155ac486c0e0b56e0bf2..99d1d553e65b7970a3ecb5158774ca5185f297a0 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -1523,6 +1523,7 @@ static inline int of_get_available_child_count(const struct device_node *np)
>   }
>   
>   #define _OF_DECLARE_STUB(table, name, compat, fn, fn_type)		\
> +	static_assert((fn) != NULL);	\
>   	static const struct of_device_id __of_table_##name		\
>   		__attribute__((unused))					\
>   		 = { .compatible = compat,				\
> @@ -1530,6 +1531,7 @@ static inline int of_get_available_child_count(const struct device_node *np)
>   
>   #if defined(CONFIG_OF) && !defined(MODULE)
>   #define _OF_DECLARE(table, name, compat, fn, fn_type)			\
> +	static_assert((fn) != NULL);	\
>   	static const struct of_device_id __of_table_##name		\
>   		__used __section("__" #table "_of_table")		\
>   		__aligned(__alignof__(struct of_device_id))		\
> 
> ---
> base-commit: 8ffd015db85fea3e15a77027fda6c02ced4d2444
> change-id: 20250417-_of_declare-ac80b59d952d
> 
> Best regards,