Re: [PATCH -v2 3/7] module/modpost: Add basename helper

Masahiro Yamada
2025-02-07T17:56:59Z

On Tue, Dec 3, 2024 at 12:11 AM Peter Zijlstra <peterz@infradead.org> wrote:

I do not think you need this helper (at least in your purpose).

You will use this helper in the 4/7 patch.
Then, you will immediately revert it in the 5/7 patch.

If you look at the final result, you did not need to add
mod_basename() in the first place.
(There is just one call site, which does not save any
duplicated code).

I suggest adding verify_module_namespace() in the 4/7 patch
instead of using the helper.

BTW, I think this helper is useful for cleaning up the current code,
but you are not even aware that this common pattern appears multiple times.

https://lore.kernel.org/all/20250207175024.412054-1-masahiroy@kernel.org/T/#u

>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> scripts/mod/modpost.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1553,6 +1553,16 @@ static void mod_set_crcs(struct module *
> free(buf);
> }
>
> +static const char *mod_basename(const char *modname)
> +{
> + const char *basename = strrchr(modname, '/');
> + if (basename)
> + basename++;
> + else
> + basename = modname;
> + return basename;
> +}
> +
> static void read_symbols(const char *modname)
> {
> const char *symname;
> @@ -1697,11 +1707,7 @@ static void check_exports(struct module
> s->crc_valid = exp->crc_valid;
> s->crc = exp->crc;
>
> - basename = strrchr(mod->name, '/');
> - if (basename)
> - basename++;
> - else
> - basename = mod->name;
> + basename = mod_basename(mod->name);
>
> if (!contains_namespace(&mod->imported_namespaces, exp->namespace)) {
> modpost_log(!allow_missing_ns_imports,
>
>

--
Best Regards

Masahiro Yamada