On Friday 28 Feb 2020 at 01:41:50 (+0900), Masahiro Yamada wrote:
> Hi.
>
> On Tue, Feb 18, 2020 at 6:41 PM Quentin Perret <qperret@xxxxxxxxxx> wrote:
> >
> > In order to prepare the ground for a build-time optimization, split
> > adjust_autoksyms.sh into two scripts: one that generates autoksyms.h
> > based on all currently available information (whitelist, and .mod
> > files), and the other to inspect the diff between two versions of
> > autoksyms.h and trigger appropriate rebuilds.
> >
> > Acked-by: Nicolas Pitre <nico@xxxxxxxxxxx>
> > Tested-by: Matthias Maennich <maennich@xxxxxxxxxx>
> > Reviewed-by: Matthias Maennich <maennich@xxxxxxxxxx>
> > Signed-off-by: Quentin Perret <qperret@xxxxxxxxxx>
> > ---
> > scripts/adjust_autoksyms.sh | 36 +++-----------------------
> > scripts/gen_autoksyms.sh | 51 +++++++++++++++++++++++++++++++++++++
> > 2 files changed, 55 insertions(+), 32 deletions(-)
> > create mode 100755 scripts/gen_autoksyms.sh
> >
> > diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
> > index ff46996525d3..2b366d945ccb 100755
> > --- a/scripts/adjust_autoksyms.sh
> > +++ b/scripts/adjust_autoksyms.sh
> > @@ -1,14 +1,13 @@
> > #!/bin/sh
> > # SPDX-License-Identifier: GPL-2.0-only
> >
> > -# Script to create/update include/generated/autoksyms.h and dependency files
> > +# Script to update include/generated/autoksyms.h and dependency files
> > #
> > # Copyright: (C) 2016 Linaro Limited
> > # Created by: Nicolas Pitre, January 2016
> > #
> >
> > -# Create/update the include/generated/autoksyms.h file from the list
> > -# of all module's needed symbols as recorded on the second line of *.mod files.
> > +# Update the include/generated/autoksyms.h file.
> > #
> > # For each symbol being added or removed, the corresponding dependency
> > # file's timestamp is updated to force a rebuild of the affected source
> > @@ -38,35 +37,8 @@ esac
> > # We need access to CONFIG_ symbols
> > . include/config/auto.conf
> >
> > -ksym_wl=/dev/null
> > -if [ -n "$CONFIG_UNUSED_KSYMS_WHITELIST" ]; then
> > - # Use 'eval' to expand the whitelist path and check if it is relative
> > - eval ksym_wl="$CONFIG_UNUSED_KSYMS_WHITELIST"
> > - [ "${ksym_wl}" != "${ksym_wl#/}" ] || ksym_wl="$abs_srctree/$ksym_wl"
> > - if [ ! -f "$ksym_wl" ]; then
>
>
> Just a Nit.
>
> Maybe, is testing -r better ?
>
> 'cat - "$ksym_wl"' is piped, so its error code is not checked.
>
> So, checking the read permission here is robust, I think.
Right, that's a good point. And actually, I think we want both -f and
-r. -r alone would consider a path to a folder as correct.
This should do the trick:
diff --git a/scripts/gen_autoksyms.sh b/scripts/gen_autoksyms.sh
index 679c9f05e4b4..16c0b2ddaa4c 100755
--- a/scripts/gen_autoksyms.sh
+++ b/scripts/gen_autoksyms.sh
@@ -24,7 +24,7 @@ if [ -n "$CONFIG_UNUSED_KSYMS_WHITELIST" ]; then
# Use 'eval' to expand the whitelist path and check if it is relative
eval ksym_wl="$CONFIG_UNUSED_KSYMS_WHITELIST"
[ "${ksym_wl}" != "${ksym_wl#/}" ] || ksym_wl="$abs_srctree/$ksym_wl"
- if [ ! -f "$ksym_wl" ]; then
+ if [ ! -f "$ksym_wl" ] || [ ! -r "$ksym_wl" ]; then
echo "ERROR: '$ksym_wl' whitelist file not found" >&2
exit 1
fi
I'll send a v6 shortly.
Thanks!
Quentin
