Re: [PATCH RFC v2 04/13] sysctl: add register_sysctl() wrapper for MODULE_SYSCTL_TABLE

[Mauricio Faria de Oliveira <mfo@igalia.com>]

On 2026-08-18 23:44, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] The macro `__UNIQUE_ID` is used without an argument in `__MODULE_SYSCTL_TABLE_NAME`, preventing its expansion.

Fixed.

> - [Low] Dangling trailing backslash on the last line of the `register_sysctl` macro definition.

Fixed.

> --
> 
> --- Patch [4]: [PATCH RFC v2 04/13] sysctl: add register_sysctl() wrapper for MODULE_SYSCTL_TABLE ---
> commit 5518fc079c95785ae760d86da6be17693ac8aeea
> Author: Mauricio Faria de Oliveira <mfo@igalia.com>
> 
> sysctl: add register_sysctl() wrapper for MODULE_SYSCTL_TABLE
> 
> Add a MODULE_SYSCTL_TABLE call into register_sysctl() for existing callers
> to automatically use it.
> 
>> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
>> index 6960fe22c50bf..7e05fafd5544e 100644
>> --- a/include/linux/sysctl.h
>> +++ b/include/linux/sysctl.h
>> @@ -335,8 +335,58 @@ struct ctl_table_root {
>>  #define MODULE_SYSCTL_TABLE(path, table)
>>  #endif
> 
> [Severity: Medium]
> Does this code prevent __UNIQUE_ID from being expanded correctly?
> 
> Looking at __MODULE_SYSCTL_TABLE_NAME in include/linux/sysctl.h:
> 
> #define __MODULE_SYSCTL_TABLE_NAME(table) \
>         __mod_device_table(sysctl, __UNIQUE_ID)
> 
> Since __UNIQUE_ID is a function-like macro that requires an argument, using it
> without parentheses prevents preprocessor expansion. This results in the
> literal string being used to construct the symbol name, which defeats the
> purpose of generating a unique identifier and could cause symbol redefinition
> errors if MODULE_SYSCTL_TABLE is used multiple times.
> 
>> -#define register_sysctl(path, table)	\
>> -	register_sysctl_sz(path, table, ARRAY_SIZE(table))
>> +/*
>> + * The register_sysctl() wrapper for the MODULE_SYSCTL_TABLE macro
> 
> [ ... ]
> 
>> +#define _register_sysctl(path, table, table_tmpl, path_tmpl)			\
>> +({										\
>> +	MODULE_SYSCTL_TABLE(path_tmpl, table_tmpl);				\
>> +	register_sysctl_sz(path, table, ARRAY_SIZE(table));			\
>> +})
>> +
>> +#define register_sysctl(path, table, tmpl_args...)				\
>> +	_register_sysctl(path, table,						\
>> +			 __sysctl_table_tmpl_or_default(table, ## tmpl_args),	\
>> +			 __sysctl_path_tmpl_or_default(path, ## tmpl_args))	\
> 
> [Severity: Low]
> Is this trailing backslash intended?
> 
> The trailing backslash on the last line of the register_sysctl macro
> definition will unnecessarily consume the empty line that follows it.
> 
>> +
>> +/* Helper macros for optional template arguments */

-- 
Mauricio