Re: [PATCH 1/2] modpost: remove useless export_from_sec()

[Nick Desaulniers]

On Sat, Apr 2, 2022 at 6:00 AM Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote:
>
> With commit 1743694eb235 ("modpost: stop symbol preloading for
> modversion CRC") applied, now export_from_sec() is useless.
>
> handle_symbol() is called for every symbol in the ELF.
>
> When 'symname' does not start with "__ksymtab", export_from_sec() is
> called, and the returned value is stored in 'export'.
>
> It is used in the last part of handle_symbol():
>
>                 if (strstarts(symname, "__ksymtab_")) {
>                         name = symname + strlen("__ksymtab_");
>                         sym_add_exported(name, mod, export);
>                 }
>
> 'export' is used only when 'symname' starts with "__ksymtab_".
>
> So, the value returned by export_from_sec() is never used.
>
> Remove this useless function. This makes further cleanups possible.
>
> Signed-off-by: Masahiro Yamada <masahiroy@xxxxxxxxxx>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>

> ---
>
>  scripts/mod/modpost.c | 17 +----------------
>  scripts/mod/modpost.h |  4 ----
>  2 files changed, 1 insertion(+), 20 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index ed9d056d2108..194ca9083c7a 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -369,16 +369,6 @@ static enum export export_from_secname(struct elf_info *elf, unsigned int sec)
>                 return export_unknown;
>  }
>
> -static enum export export_from_sec(struct elf_info *elf, unsigned int sec)
> -{
> -       if (sec == elf->export_sec)
> -               return export_plain;
> -       else if (sec == elf->export_gpl_sec)
> -               return export_gpl;
> -       else
> -               return export_unknown;
> -}
> -
>  static const char *namespace_from_kstrtabns(const struct elf_info *info,
>                                             const Elf_Sym *sym)
>  {
> @@ -576,10 +566,7 @@ static int parse_elf(struct elf_info *info, const char *filename)
>                                 fatal("%s has NOBITS .modinfo\n", filename);
>                         info->modinfo = (void *)hdr + sechdrs[i].sh_offset;
>                         info->modinfo_len = sechdrs[i].sh_size;
> -               } else if (strcmp(secname, "__ksymtab") == 0)
> -                       info->export_sec = i;
> -               else if (strcmp(secname, "__ksymtab_gpl") == 0)
> -                       info->export_gpl_sec = i;
> +               }
>
>                 if (sechdrs[i].sh_type == SHT_SYMTAB) {
>                         unsigned int sh_link_idx;
> @@ -702,8 +689,6 @@ static void handle_symbol(struct module *mod, struct elf_info *info,
>
>         if (strstarts(symname, "__ksymtab"))
>                 export = export_from_secname(info, get_secindex(info, sym));
> -       else
> -               export = export_from_sec(info, get_secindex(info, sym));
>
>         switch (sym->st_shndx) {
>         case SHN_COMMON:
> diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
> index 0c47ff95c0e2..a85dcec3669a 100644
> --- a/scripts/mod/modpost.h
> +++ b/scripts/mod/modpost.h
> @@ -25,7 +25,6 @@
>  #define Elf_Sym     Elf32_Sym
>  #define Elf_Addr    Elf32_Addr
>  #define Elf_Sword   Elf64_Sword
> -#define Elf_Section Elf32_Half
>  #define ELF_ST_BIND ELF32_ST_BIND
>  #define ELF_ST_TYPE ELF32_ST_TYPE
>
> @@ -40,7 +39,6 @@
>  #define Elf_Sym     Elf64_Sym
>  #define Elf_Addr    Elf64_Addr
>  #define Elf_Sword   Elf64_Sxword
> -#define Elf_Section Elf64_Half
>  #define ELF_ST_BIND ELF64_ST_BIND
>  #define ELF_ST_TYPE ELF64_ST_TYPE
>
> @@ -138,8 +136,6 @@ struct elf_info {
>         Elf_Shdr     *sechdrs;
>         Elf_Sym      *symtab_start;
>         Elf_Sym      *symtab_stop;
> -       Elf_Section  export_sec;
> -       Elf_Section  export_gpl_sec;
>         char         *strtab;
>         char         *modinfo;
>         unsigned int modinfo_len;
> --
> 2.32.0
>


-- 
Thanks,
~Nick Desaulniers