lkml.org 
[lkml]   [2020]   [Feb]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH v5 2/3] kbuild: split adjust_autoksyms.sh in two parts
    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@google.com> 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@fluxnic.net>
    > > Tested-by: Matthias Maennich <maennich@google.com>
    > > Reviewed-by: Matthias Maennich <maennich@google.com>
    > > Signed-off-by: Quentin Perret <qperret@google.com>
    > > ---
    > > 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

    \
     
     \ /
      Last update: 2020-02-28 11:03    [W:5.482 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site