On Sun, Apr 24, 2022 at 12:09 PM Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote:
>
> Add a small helper, sym_add_unresolved() to ease the further
> refactoring.
>
> Remove the 'weak' argument from alloc_symbol() because it is sensible
> only for unresolved symbols.
I did not yet read the rest of the series to see how else your newly
added helper `sym_add_unresolved` is used.
Perhaps the callers of `alloc_symbol` should just set the symbol's
weak member to true if needed, and alloc_symbol can default to setting
it false (as the memset currently does)?
Then, you don't need the helper, and just `handle_symbol` needs the
assignment when `ELF_ST_BIND(sym->st_info) == STB_WEAK`?
>
> Signed-off-by: Masahiro Yamada <masahiroy@xxxxxxxxxx>
> ---
>
> scripts/mod/modpost.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 86416e4af626..1c7d2831e89d 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -230,14 +230,12 @@ 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, bool weak,
> - struct symbol *next)
> +static struct symbol *alloc_symbol(const char *name, struct symbol *next)
> {
> struct symbol *s = NOFAIL(malloc(sizeof(*s) + strlen(name) + 1));
>
> memset(s, 0, sizeof(*s));
> strcpy(s->name, name);
> - s->weak = weak;
> s->next = next;
> s->is_static = true;
> return s;
> @@ -250,11 +248,17 @@ static struct symbol *new_symbol(const char *name, struct module *module,
> unsigned int hash;
>
> hash = tdb_hash(name) % SYMBOL_HASH_SIZE;
> - symbolhash[hash] = alloc_symbol(name, false, symbolhash[hash]);
> + symbolhash[hash] = alloc_symbol(name, symbolhash[hash]);
>
> return symbolhash[hash];
> }
>
> +static void sym_add_unresolved(const char *name, struct module *mod, bool weak)
> +{
> + mod->unres = alloc_symbol(name, mod->unres);
> + mod->unres->weak = weak;
> +}
> +
> static struct symbol *find_symbol(const char *name)
> {
> struct symbol *s;
> @@ -701,9 +705,8 @@ static void handle_symbol(struct module *mod, struct elf_info *info,
> }
> }
>
> - mod->unres = alloc_symbol(symname,
> - ELF_ST_BIND(sym->st_info) == STB_WEAK,
> - mod->unres);
> + sym_add_unresolved(symname, mod,
> + ELF_ST_BIND(sym->st_info) == STB_WEAK);
> break;
> default:
> /* All exported symbols */
> @@ -2073,7 +2076,7 @@ static void read_symbols(const char *modname)
> * the automatic versioning doesn't pick it up, but it's really
> * important anyhow */
> if (modversions)
> - mod->unres = alloc_symbol("module_layout", false, mod->unres);
> + sym_add_unresolved("module_layout", mod, false);
> }
>
> static void read_symbols_from_files(const char *filename)
> --
> 2.32.0
>
--
Thanks,
~Nick Desaulniers
