Re: [PATCH v4] kbuild: rewrite check-local-export in sh/awk

[Masahiro Yamada <masahiroy@kernel.org>]

On Sun, Sep 4, 2022 at 1:01 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Thu, Sep 1, 2022 at 6:03 AM Owen Rafferty <owen@owenrafferty.com> wrote:
> >
> > Signed-off-by: Owen Rafferty <owen@owenrafferty.com>
> > ---
>
>
> Please input something in the commit log.
>
> I think the benchmark in v2 is worth mentioning
> because "awk is faster than bash" is one benefit
> of applying this patch.
>
>



Applied to linux-kbuild. Thanks.


(V5 was not delivered to ML somehow,
but I found it in my mailbox.)









>
>
>
>
> >  scripts/check-local-export | 96 +++++++++++++++++++-------------------
> >  1 file changed, 47 insertions(+), 49 deletions(-)
> >
> > diff --git a/scripts/check-local-export b/scripts/check-local-export
> > index 6ccc2f467416..0c049ff44aca 100755
> > --- a/scripts/check-local-export
> > +++ b/scripts/check-local-export
> > @@ -1,26 +1,14 @@
> > -#!/usr/bin/env bash
> > +#!/bin/sh
> >  # SPDX-License-Identifier: GPL-2.0-only
> >  #
> >  # Copyright (C) 2022 Masahiro Yamada <masahiroy@kernel.org>
> > +# Copyright (C) 2022 Owen Rafferty <owen@owenrafferty.com>
> >  #
> >  # Exit with error if a local exported symbol is found.
> >  # EXPORT_SYMBOL should be used for global symbols.
> >
> >  set -e
> >
> > -# catch errors from ${NM}
> > -set -o pipefail
> > -
> > -# Run the last element of a pipeline in the current shell.
> > -# Without this, the while-loop would be executed in a subshell, and
> > -# the changes made to 'symbol_types' and 'export_symbols' would be lost.
> > -shopt -s lastpipe
> > -
> > -declare -A symbol_types
> > -declare -a export_symbols
> > -
> > -exit_code=0
> > -
> >  # If there is no symbol in the object, ${NM} (both GNU nm and llvm-nm) shows
> >  # 'no symbols' diagnostic (but exits with 0). It is harmless and hidden by
> >  # '2>/dev/null'. However, it suppresses real error messages as well. Add a
> > @@ -29,43 +17,53 @@ exit_code=0
> >  # TODO:
> >  # Use --quiet instead of 2>/dev/null when we upgrade the minimum version of
> >  # binutils to 2.37, llvm to 13.0.0.
> > -# Then, the following line will be really simple:
> > -#   ${NM} --quiet ${1} |
> > +# Then, the following line will be simpler:
> > +#   { ${NM} --quiet ${1} || kill 0; } |
> > +
> > +{ ${NM} ${1} 2>/dev/null || { echo "${0}: ${NM} failed" >&2; kill 0; } } |
> > +${AWK} -v "file=${1}" '
> > +BEGIN {
> > +       i = 0
> > +}
> > +
> > +# Skip the line if the number of fields is less than 3.
> > +#
> > +# case 1)
> > +#   For undefined symbols, the first field (value) is empty.
> > +#   The outout looks like this:
> > +#     "                 U _printk"
> > +#   It is unneeded to record undefined symbols.
> > +#
> > +# case 2)
> > +#   For Clang LTO, llvm-nm outputs a line with type t but empty name:
> > +#     "---------------- t"
> > +!length($3) {
> > +       next
> > +}
> >
> > -{ ${NM} ${1} 2>/dev/null || { echo "${0}: ${NM} failed" >&2; false; } } |
> > -while read value type name
> > -do
> > -       # Skip the line if the number of fields is less than 3.
> > -       #
> > -       # case 1)
> > -       #   For undefined symbols, the first field (value) is empty.
> > -       #   The outout looks like this:
> > -       #     "                 U _printk"
> > -       #   It is unneeded to record undefined symbols.
> > -       #
> > -       # case 2)
> > -       #   For Clang LTO, llvm-nm outputs a line with type 't' but empty name:
> > -       #     "---------------- t"
> > -       if [[ -z ${name} ]]; then
> > -               continue
> > -       fi
> > +# save (name, type) in the associative array
> > +{ symbol_types[$3]=$2 }
> >
> > -       # save (name, type) in the associative array
> > -       symbol_types[${name}]=${type}
> > +# append the exported symbol to the array
> > +($3 ~ /^__ksymtab_/) {
> > +       export_symbols[i] = $3
> > +       sub(/^__ksymtab_/, "", export_symbols[i])
> > +       i++
> > +}
> >
> > -       # append the exported symbol to the array
> > -       if [[ ${name} == __ksymtab_* ]]; then
> > -               export_symbols+=(${name#__ksymtab_})
> > -       fi
> > -done
> > +END {
> > +       exit_code = 0
> > +       for (j = 0; j < i; ++j) {
> > +               name = export_symbols[j]
> > +               # nm(3) says "If lowercase, the symbol is usually local"
> > +               if (symbol_types[name] ~ /[a-z]/) {
> > +                       printf "%s: error: local symbol %s was exported\n",
> > +                               file, name | "cat 1>&2"
> > +                       exit_code = 1
> > +               }
> > +       }
> >
> > -for name in "${export_symbols[@]}"
> > -do
> > -       # nm(3) says "If lowercase, the symbol is usually local"
> > -       if [[ ${symbol_types[$name]} =~ [a-z] ]]; then
> > -               echo "$@: error: local symbol '${name}' was exported" >&2
> > -               exit_code=1
> > -       fi
> > -done
> > +       exit exit_code
> > +}'
> >
> > -exit ${exit_code}
> > +exit $?
> > --
> > 2.37.3
> >
>
>
> --
> Best Regards
> Masahiro Yamada



--
Best Regards
Masahiro Yamada