lkml.org 
[lkml]   [2018]   [Jul]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 3/4] perf tools: Fix check-headers.sh output file variables
    On Fri, Jul 20, 2018 at 06:22:49PM +0300, Alexander Kapshuk wrote:
    > On Fri, Jul 20, 2018 at 6:16 PM Jiri Olsa <jolsa@redhat.com> wrote:
    > >
    > > On Fri, Jul 20, 2018 at 11:57:45AM -0300, Arnaldo Carvalho de Melo wrote:
    > > > Em Fri, Jul 20, 2018 at 01:00:35PM +0200, Jiri Olsa escreveu:
    > > > > The warning message in check_w function uses wrongly
    > > > > the $file variable instead of $file1 and $file2.
    > > >
    > > > Humm,
    > > >
    > > > Before:
    > > >
    > > > Warning: Kernel ABI header at 'tools/arch/powerpc/include/uapi/asm/unistd.h' differs from latest version at 'arch/powerpc/include/uapi/asm/unistd.h'
    > > >
    > > > After:
    > > >
    > > > Warning: Kernel ABI header at '../arch/powerpc/include/uapi/asm/unistd.h' differs from latest version at '../../arch/powerpc/include/uapi/asm/unistd.h'
    > > >
    > > >
    > > > The previous version is better, I can then just use:
    > > >
    > > > diff -u tools/arch/powerpc/include/uapi/asm/unistd.h arch/powerpc/include/uapi/asm/unistd.h
    > > >
    > > > and get what changed, with your change I have to go to tools/perf before
    > > > doing that diff, which is an unnecessary extra step in at least my
    > > > workflow.
    > >
    > > so all paths output based in kernel tree root then, will change
    > >
    > > jirka
    >
    > I was going to ask about this in a separate email initially, but then
    > thought I'd use this email exchange instead, as my question is about
    > the code in question. Hope you don't mind.
    >
    > If I'm reading this right, the intended behavoir of the block of code
    > below is to test file2 for existance, and if it exists, to evaluate $cmd.
    > If file1 and file2 are found to differ, print the warning.
    >
    > test -f $file2 &&
    > eval $cmd || echo "Warning: Kernel ABI header at 'tools/$file'
    > differs from latest version at '$file'" >&2
    >
    > The '||' path of execution is however also taken if file2 doesn't exist,
    > which is probably very unlikely to happen. See below.
    >
    > % file1=file1; file2=file2
    > % cmd="echo diff $file1 $file2"
    > % test -f $file2 &&
    > eval $cmd || echo "Warning: Kernel ABI header at 'tools/$file1'
    > differs from latest version at '$file2'" >&2
    > Warning: Kernel ABI header at 'tools/file1' differs from latest
    > version at 'file2'
    >
    > Is this something you would rather leave as is, or perhaps use something
    > along the lines of the code below instead:
    >
    > test -f $file2 && {
    > eval $cmd ||
    > echo "Warning: Kernel ABI header at 'tools/$file' differs from latest
    > version at '$file'" >&2
    > }

    hi,
    yea, probably.. please feel free to post a patch.. just make sure all
    the displayed files paths are based on the kernel root

    thanks,
    jirka

    \
     
     \ /
      Last update: 2018-07-23 09:02    [W:2.142 / U:0.088 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site