Re: [PATCH v2 13/26] modpost: split new_symbol() to symbol allocation and hash table addition

Nick Desaulniers
2022-05-03T22:01:17Z

On Sun, May 1, 2022 at 1:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> new_symbol() does two things; allocate a new symbol and register it
> to the hash table.
>
> Using a separate function for each is easier to understand.
>
> Replace new_symbol() with hash_add_symbol(). Remove the second parameter
> of alloc_symbol().
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
> Changes in v2:
> - New patch
>
> scripts/mod/modpost.c | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index c9b75697d0fc..b9f359d10968 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -242,34 +242,31 @@ static inline unsigned int tdb_hash(const char *name)
> * Allocate a new symbols for use in the hash of exported symbols or
> * the list of unresolved symbols per module
> **/
> -static struct symbol *alloc_symbol(const char *name, struct symbol *next)
> +static struct symbol *alloc_symbol(const char *name)
> {
> struct symbol *s = NOFAIL(malloc(sizeof(*s) + strlen(name) + 1));
>
> memset(s, 0, sizeof(*s));
> strcpy(s->name, name);
> - s->next = next;
> s->is_static = true;
> return s;
> }
>
> /* For the hash of exported symbols */
> -static struct symbol *new_symbol(const char *name, struct module *module,
> - enum export export)

`module` was also previously unused! Yuck. Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> +static void hash_add_symbol(struct symbol *sym)
> {
> unsigned int hash;
>
> - hash = tdb_hash(name) % SYMBOL_HASH_SIZE;
> - symbolhash[hash] = alloc_symbol(name, symbolhash[hash]);
> -
> - return symbolhash[hash];
> + hash = tdb_hash(sym->name) % SYMBOL_HASH_SIZE;
> + sym->next = symbolhash[hash];
> + symbolhash[hash] = sym;
> }
>
> static void sym_add_unresolved(const char *name, struct module *mod, bool weak)
> {
> struct symbol *sym;
>
> - sym = alloc_symbol(name, NULL);
> + sym = alloc_symbol(name);
> sym->weak = weak;
>
> list_add_tail(&sym->list, &mod->unresolved_symbols);
> @@ -418,10 +415,11 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod,
> s->module->is_vmlinux ? "" : ".ko");
> }
>
> - s = new_symbol(name, mod, export);
> + s = alloc_symbol(name);
> s->module = mod;
> s->export = export;
> list_add_tail(&s->list, &mod->exported_symbols);
> + hash_add_symbol(s);
>
> return s;
> }
> --
> 2.32.0
>

--
Thanks,
~Nick Desaulniers