Re: [PATCH RFC v2 06/13] sysctl, net: add register_net_sysctl{_sz}() wrappers for MODULE_SYSCTL_TABLE

On 2026-08-18 23:40, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] When `CONFIG_SYSCTL` is disabled, the build fails due to a preprocessor conflict where the newly introduced `register_net_sysctl_sz` macro expands inside its own fallback static inline function definition.

Fixed.

> --
> 
> --- Patch [6]: [PATCH RFC v2 06/13] sysctl, net: add register_net_sysctl{_sz}() wrappers for MODULE_SYSCTL_TABLE ---
> commit fad97277609904f7e692d6e11a61fdff7e4ff635
> Author: Mauricio Faria de Oliveira <[email protected]>
> 
> sysctl, net: add register_net_sysctl{_sz}() wrappers for MODULE_SYSCTL_TABLE
> 
> This commit replaces register_net_sysctl() and register_net_sysctl_sz() with
> macros that automatically use MODULE_SYSCTL_TABLE for existing callers. It
> renames the actual sysctl registration function to __register_net_sysctl_sz()
> and adds optional template arguments to support dynamically defined paths and
> tables.
> 
>> diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
>> index e5ee673b9fcf8..2127049c71ced 100644
>> --- a/include/net/net_namespace.h
>> +++ b/include/net/net_namespace.h
> [ ... ]
>> @@ -520,13 +520,48 @@ void unregister_pernet_device(struct pernet_operations *);
>> 
>>  struct ctl_table;
>> 
>> -#define register_net_sysctl(net, path, table)	\
>> -	register_net_sysctl_sz(net, path, table, ARRAY_SIZE(table))
>> +/*
>> + * The register_net_sysctl{_sz}() wrappers for MODULE_SYSCTL_TABLE
>> + * automatically create symbols in sysctl registration sites.
> [ ... ]
>> + */
>> +#define _register_net_sysctl_sz(net, path, table, size, table_tmpl, path_tmpl)	\
>> +({										\
>> +	MODULE_SYSCTL_TABLE(path_tmpl, table_tmpl);				\
>> +	__register_net_sysctl_sz(net, path, table, size);			\
>> +})
>> +
>> +#define register_net_sysctl_sz(net, path, table, size, tmpl_args...)		\
>> +	_register_net_sysctl_sz(net, path, table, size,				\
>> +				__sysctl_table_tmpl_or_default(table,		\
>> +							       ## tmpl_args),	\
>> +				__sysctl_path_tmpl_or_default(path,		\
>> +							      ## tmpl_args))
>> +#define register_net_sysctl(net, path, table, tmpl_args...)			\
>> +	register_net_sysctl_sz(net, path, table,				\
>> +			       __sysctl_table_array_size(table, ## tmpl_args),	\
>> +			       ## tmpl_args)
>> +
>> +/* Helper macro for optional template arguments */
>> +#define __sysctl_table_array_size(table, tmpl_args...)	\
>> +	ARRAY_SIZE(__sysctl_table_tmpl_or_default(table, ## tmpl_args))
>> +
>>  #ifdef CONFIG_SYSCTL
>>  int net_sysctl_init(void);
>> -struct ctl_table_header *register_net_sysctl_sz(struct net *net, const char *path,
>> -						const struct ctl_table *table,
>> -						size_t table_size);
>> +struct ctl_table_header *__register_net_sysctl_sz(struct net *net,
>> +						  const char *path,
>> +						  const struct ctl_table *table,
>> +						  size_t table_size);
>>  void unregister_net_sysctl_table(struct ctl_table_header *header);
>>  #else
>>  static inline int net_sysctl_init(void) { return 0; }
> 
> [Severity: Low]
> Does this commit break the build when CONFIG_SYSCTL is disabled?
> 
> Because register_net_sysctl_sz() is now defined as a macro unconditionally
> above, the fallback static inline function in this #else block:
> 
> static inline struct ctl_table_header *register_net_sysctl_sz(struct net *net,
> 	const char *path, const struct ctl_table *table, size_t table_size)
> {
> 	return NULL;
> }
> 
> will cause a macro expansion conflict.
> 
> Should this static inline function be renamed to __register_net_sysctl_sz()
> to avoid expanding the macro in its own definition and to provide the fallback
> implementation needed by the new macro?

-- 
Mauricio