Re: [PATCH RFC v2 07/13] sysctl, net: update register_net_sysctl{_sz}() callers with template arguments

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Syntax error and implicit function declaration when CONFIG_SYSCTL is disabled due to macro collision.
- [Low] The `__UNIQUE_ID` macro is used without arguments in `__MODULE_SYSCTL_TABLE_NAME`, preventing it from generating a unique identifier.
--

--- Patch [7]: [PATCH RFC v2 07/13] sysctl, net: update register_net_sysctl{_sz}() callers with template arguments ---
commit 35e3405d9382d3cfe84c302ce928023b61da151b
Author: Mauricio Faria de Oliveira <[email protected]>

sysctl, net: update register_net_sysctl{_sz}() callers with template arguments

Update most callers with dynamic table/path to use template arguments.

> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 1349c0eedb642..bdd67dd879d24 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -12,6 +12,7 @@
>   */
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +#define SYSCTL_MODULE_ALIASES_UNIQUE_ID

[Severity: Low]
When SYSCTL_MODULE_ALIASES_UNIQUE_ID is defined here, the macro
__MODULE_SYSCTL_TABLE_NAME(table) in include/linux/sysctl.h expands to
__mod_device_table(sysctl, __UNIQUE_ID).

Since __UNIQUE_ID is a function-like macro requiring a prefix argument, does
omitting the parentheses prevent it from being expanded by the preprocessor?
Could this create identical literal symbol names for all instances in a module
and defeat the uniqueness mechanism?

>  
>  #include <linux/slab.h>
>  #include <linux/types.h>
[ ... ]
> @@ -3910,11 +3911,15 @@ int neigh_sysctl_register(struct net_device *dev, struct neigh_parms *p,
>  	      BUG();
>  	}
>  
> -	snprintf(neigh_path, sizeof(neigh_path), "net/%s/neigh/%s",
> -		p_name, dev_name_source);
> +#define path_template "net/%s/neigh/%s"
> +	snprintf(neigh_path, sizeof(neigh_path), path_template,
> +		 p_name, dev_name_source);
>  	t->sysctl_header = register_net_sysctl_sz(neigh_parms_net(p),
>  						  neigh_path, t->neigh_vars,
> -						  neigh_vars_size);
> +						  neigh_vars_size,
> +						  neigh_sysctl_template.neigh_vars,
> +						  path_template);

[Severity: Low]
Does this call to register_net_sysctl_sz() cause a build failure when
CONFIG_SYSCTL is disabled?

It looks like register_net_sysctl_sz is globally defined as a macro, which
unconditionally expands over the static inline fallback definition in
include/net/net_namespace.h, producing invalid C syntax. Additionally, would
this macro expand to __register_net_sysctl_sz(), which lacks a stub in the
!CONFIG_SYSCTL path, causing implicit function declaration errors?

> +#undef path_template
>  	if (!t->sysctl_header)
>  		goto free;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7