Re: [PATCH RFC v2 07/13] sysctl, net: update register_net_sysctl{_sz}() callers with template arguments
Mauricio Faria de Oliveira <[email protected]>
| Newsgroups | dev.linux.lists.mptcp,dev.linux.lists.sashiko-reviews,org.kernel.vger.bpf,org.kernel.vger.linux-s390 |
|---|---|
| Message-ID | <[email protected]> |
On 2026-08-18 23:48, [email protected] wrote: > 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. Both fixed previously. > -- > > --- 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; -- Mauricio