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

[Nick Desaulniers]

On Sun, May 1, 2022 at 1:42 AM Masahiro Yamada <masahiroy@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
>
> 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@xxxxxxxxxx>

> +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