| Message ID | 158920418730.16156.8299185499520876735.stgit@devnote2 (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 8e923a2168afd221ea26e3d9716f21e9578b5c4d |
| Headers | show |
| Series | [v2] selftests/ftrace: Use printf for backslash included command | expand |
Hi Andreas and David, OK, what about this fix? On Mon, 11 May 2020 22:36:27 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote: > Since the built-in echo has different behavior in POSIX shell > (dash) and bash, kprobe_syntax_errors.tc can fail on dash which > interpret backslash escape automatically. > > To fix this issue, we explicitly use printf "%s" (not interpret > backslash escapes) if the command string can include backslash. > > Reported-by: Liu Yiding <yidingx.liu@intel.com> > Suggested-by: Xiao Yang <yangx.jy@cn.fujitsu.com> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > --- > tools/testing/selftests/ftrace/test.d/functions | 8 +++++--- > .../ftrace/test.d/kprobe/kprobe_syntax_errors.tc | 4 +++- > 2 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/tools/testing/selftests/ftrace/test.d/functions b/tools/testing/selftests/ftrace/test.d/functions > index 61a3c7e2634d..697c77ef2e2b 100644 > --- a/tools/testing/selftests/ftrace/test.d/functions > +++ b/tools/testing/selftests/ftrace/test.d/functions > @@ -119,12 +119,14 @@ yield() { > ping $LOCALHOST -c 1 || sleep .001 || usleep 1 || sleep 1 > } > > +# Since probe event command may include backslash, explicitly use printf "%s" > +# to NOT interpret it. > ftrace_errlog_check() { # err-prefix command-with-error-pos-by-^ command-file > - pos=$(echo -n "${2%^*}" | wc -c) # error position > - command=$(echo "$2" | tr -d ^) > + pos=$(printf "%s" "${2%^*}" | wc -c) # error position > + command=$(printf "%s" "$2" | tr -d ^) > echo "Test command: $command" > echo > error_log > - (! echo "$command" >> "$3" ) 2> /dev/null > + (! printf "%s" "$command" >> "$3" ) 2> /dev/null > grep "$1: error:" -A 3 error_log > N=$(tail -n 1 error_log | wc -c) > # " Command: " and "^\n" => 13 > diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc > index ef1e9bafb098..eb0f4ab4e070 100644 > --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc > +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc > @@ -91,7 +91,9 @@ esac > if grep -q "Create/append/" README && grep -q "imm-value" README; then > echo 'p:kprobes/testevent _do_fork' > kprobe_events > check_error '^r:kprobes/testevent do_exit' # DIFF_PROBE_TYPE > -echo 'p:kprobes/testevent _do_fork abcd=\1' > kprobe_events > + > +# Explicitly use printf "%s" to not interpret \1 > +printf "%s" 'p:kprobes/testevent _do_fork abcd=\1' > kprobe_events > check_error 'p:kprobes/testevent _do_fork ^bcd=\1' # DIFF_ARG_TYPE > check_error 'p:kprobes/testevent _do_fork ^abcd=\1:u8' # DIFF_ARG_TYPE > check_error 'p:kprobes/testevent _do_fork ^abcd=\"foo"' # DIFF_ARG_TYPE >
On Mai 11 2020, Masami Hiramatsu wrote: > - (! echo "$command" >> "$3" ) 2> /dev/null > + (! printf "%s" "$command" >> "$3" ) 2> /dev/null printf %s does not print a newline, you need printf '%s\n' for that. Andreas.
From: Masami Hiramatsu > Sent: 11 May 2020 14:38 > > Hi Andreas and David, > > OK, what about this fix? No idea what it is trying to do or why. Just a way of avoiding the differences between SYSV and BSD /bin/echo. IIRC Posix allows both behaviours (and probably others). David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Mon, 11 May 2020 15:42:10 +0200 Andreas Schwab <schwab@linux-m68k.org> wrote: > On Mai 11 2020, Masami Hiramatsu wrote: > > > - (! echo "$command" >> "$3" ) 2> /dev/null > > + (! printf "%s" "$command" >> "$3" ) 2> /dev/null > > printf %s does not print a newline, you need printf '%s\n' for that. Actually, ftrace doesn't need newline for single command. The reason why we had used echo instead of "echo -n" here, is just for short typing :) Thank you,
On Mon, 11 May 2020 13:46:35 +0000 David Laight <David.Laight@ACULAB.COM> wrote: > From: Masami Hiramatsu > > Sent: 11 May 2020 14:38 > > > > Hi Andreas and David, > > > > OK, what about this fix? > > No idea what it is trying to do or why. > Just a way of avoiding the differences between SYSV and BSD /bin/echo. > > IIRC Posix allows both behaviours (and probably others). Ah, I got it. That's why POSIX said "the results are implementation-defined." https://pubs.opengroup.org/onlinepubs/009695399/utilities/echo.html Thank you!
> > + pos=$(printf "%s" "${2%^*}" | wc -c) # error position > > + command=$(printf "%s" "$2" | tr -d ^) You may want to put all the $(...) inside "" to avoid field splitting (not relevant to a shell assignment with modern shells) and filename globbing. > > echo "Test command: $command" > > echo > error_log > > - (! echo "$command" >> "$3" ) 2> /dev/null > > + (! printf "%s" "$command" >> "$3" ) 2> /dev/null WTF is the (! for ?? The (...) is a subshell. And ! inverts the exit status. Neither is needed at all. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Mon, 11 May 2020 14:59:20 +0000 David Laight <David.Laight@ACULAB.COM> wrote: > > > echo "Test command: $command" > > > echo > error_log > > > - (! echo "$command" >> "$3" ) 2> /dev/null > > > + (! printf "%s" "$command" >> "$3" ) 2> /dev/null > > WTF is the (! for ?? > The (...) is a subshell. > And ! inverts the exit status. > Neither is needed at all. This is done because the scripts are run with '-e' and will exit the script on any error. This particular test is examining errors in the error log. The command being written into $3 is going to fail, and return an exit code. The "(! ..)" is needed, otherwise that failure will exit out the script. -- Steve
Hi Shuah, Could you pick this to kselftest-next? Thank you, On Mon, 11 May 2020 22:36:27 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote: > Since the built-in echo has different behavior in POSIX shell > (dash) and bash, kprobe_syntax_errors.tc can fail on dash which > interpret backslash escape automatically. > > To fix this issue, we explicitly use printf "%s" (not interpret > backslash escapes) if the command string can include backslash. > > Reported-by: Liu Yiding <yidingx.liu@intel.com> > Suggested-by: Xiao Yang <yangx.jy@cn.fujitsu.com> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > --- > tools/testing/selftests/ftrace/test.d/functions | 8 +++++--- > .../ftrace/test.d/kprobe/kprobe_syntax_errors.tc | 4 +++- > 2 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/tools/testing/selftests/ftrace/test.d/functions b/tools/testing/selftests/ftrace/test.d/functions > index 61a3c7e2634d..697c77ef2e2b 100644 > --- a/tools/testing/selftests/ftrace/test.d/functions > +++ b/tools/testing/selftests/ftrace/test.d/functions > @@ -119,12 +119,14 @@ yield() { > ping $LOCALHOST -c 1 || sleep .001 || usleep 1 || sleep 1 > } > > +# Since probe event command may include backslash, explicitly use printf "%s" > +# to NOT interpret it. > ftrace_errlog_check() { # err-prefix command-with-error-pos-by-^ command-file > - pos=$(echo -n "${2%^*}" | wc -c) # error position > - command=$(echo "$2" | tr -d ^) > + pos=$(printf "%s" "${2%^*}" | wc -c) # error position > + command=$(printf "%s" "$2" | tr -d ^) > echo "Test command: $command" > echo > error_log > - (! echo "$command" >> "$3" ) 2> /dev/null > + (! printf "%s" "$command" >> "$3" ) 2> /dev/null > grep "$1: error:" -A 3 error_log > N=$(tail -n 1 error_log | wc -c) > # " Command: " and "^\n" => 13 > diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc > index ef1e9bafb098..eb0f4ab4e070 100644 > --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc > +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc > @@ -91,7 +91,9 @@ esac > if grep -q "Create/append/" README && grep -q "imm-value" README; then > echo 'p:kprobes/testevent _do_fork' > kprobe_events > check_error '^r:kprobes/testevent do_exit' # DIFF_PROBE_TYPE > -echo 'p:kprobes/testevent _do_fork abcd=\1' > kprobe_events > + > +# Explicitly use printf "%s" to not interpret \1 > +printf "%s" 'p:kprobes/testevent _do_fork abcd=\1' > kprobe_events > check_error 'p:kprobes/testevent _do_fork ^bcd=\1' # DIFF_ARG_TYPE > check_error 'p:kprobes/testevent _do_fork ^abcd=\1:u8' # DIFF_ARG_TYPE > check_error 'p:kprobes/testevent _do_fork ^abcd=\"foo"' # DIFF_ARG_TYPE >
On 5/25/20 3:59 AM, Masami Hiramatsu wrote: > Hi Shuah, > > Could you pick this to kselftest-next? > > Thank you, > > On Mon, 11 May 2020 22:36:27 +0900 > Masami Hiramatsu <mhiramat@kernel.org> wrote: > >> Since the built-in echo has different behavior in POSIX shell >> (dash) and bash, kprobe_syntax_errors.tc can fail on dash which >> interpret backslash escape automatically. >> >> To fix this issue, we explicitly use printf "%s" (not interpret >> backslash escapes) if the command string can include backslash. >> >> Reported-by: Liu Yiding <yidingx.liu@intel.com> >> Suggested-by: Xiao Yang <yangx.jy@cn.fujitsu.com> >> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> >> --- >> tools/testing/selftests/ftrace/test.d/functions | 8 +++++--- >> .../ftrace/test.d/kprobe/kprobe_syntax_errors.tc | 4 +++- >> 2 files changed, 8 insertions(+), 4 deletions(-) >> >> diff --git a/tools/testing/selftests/ftrace/test.d/functions b/tools/testing/selftests/ftrace/test.d/functions >> index 61a3c7e2634d..697c77ef2e2b 100644 >> --- a/tools/testing/selftests/ftrace/test.d/functions >> +++ b/tools/testing/selftests/ftrace/test.d/functions >> @@ -119,12 +119,14 @@ yield() { >> ping $LOCALHOST -c 1 || sleep .001 || usleep 1 || sleep 1 >> } >> >> +# Since probe event command may include backslash, explicitly use printf "%s" >> +# to NOT interpret it. >> ftrace_errlog_check() { # err-prefix command-with-error-pos-by-^ command-file >> - pos=$(echo -n "${2%^*}" | wc -c) # error position >> - command=$(echo "$2" | tr -d ^) >> + pos=$(printf "%s" "${2%^*}" | wc -c) # error position >> + command=$(printf "%s" "$2" | tr -d ^) >> echo "Test command: $command" >> echo > error_log >> - (! echo "$command" >> "$3" ) 2> /dev/null >> + (! printf "%s" "$command" >> "$3" ) 2> /dev/null >> grep "$1: error:" -A 3 error_log >> N=$(tail -n 1 error_log | wc -c) >> # " Command: " and "^\n" => 13 >> diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc >> index ef1e9bafb098..eb0f4ab4e070 100644 >> --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc >> +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc >> @@ -91,7 +91,9 @@ esac >> if grep -q "Create/append/" README && grep -q "imm-value" README; then >> echo 'p:kprobes/testevent _do_fork' > kprobe_events >> check_error '^r:kprobes/testevent do_exit' # DIFF_PROBE_TYPE >> -echo 'p:kprobes/testevent _do_fork abcd=\1' > kprobe_events >> + >> +# Explicitly use printf "%s" to not interpret \1 >> +printf "%s" 'p:kprobes/testevent _do_fork abcd=\1' > kprobe_events >> check_error 'p:kprobes/testevent _do_fork ^bcd=\1' # DIFF_ARG_TYPE >> check_error 'p:kprobes/testevent _do_fork ^abcd=\1:u8' # DIFF_ARG_TYPE >> check_error 'p:kprobes/testevent _do_fork ^abcd=\"foo"' # DIFF_ARG_TYPE >> > > Applied to git.kernel.org/pub/scm/linux/kernel/git/shuah/linux kselftest.git/ next branch for Linux 5.8-rc1 thanks, -- Shuah
diff --git a/tools/testing/selftests/ftrace/test.d/functions b/tools/testing/selftests/ftrace/test.d/functions index 61a3c7e2634d..697c77ef2e2b 100644 --- a/tools/testing/selftests/ftrace/test.d/functions +++ b/tools/testing/selftests/ftrace/test.d/functions @@ -119,12 +119,14 @@ yield() { ping $LOCALHOST -c 1 || sleep .001 || usleep 1 || sleep 1 } +# Since probe event command may include backslash, explicitly use printf "%s" +# to NOT interpret it. ftrace_errlog_check() { # err-prefix command-with-error-pos-by-^ command-file - pos=$(echo -n "${2%^*}" | wc -c) # error position - command=$(echo "$2" | tr -d ^) + pos=$(printf "%s" "${2%^*}" | wc -c) # error position + command=$(printf "%s" "$2" | tr -d ^) echo "Test command: $command" echo > error_log - (! echo "$command" >> "$3" ) 2> /dev/null + (! printf "%s" "$command" >> "$3" ) 2> /dev/null grep "$1: error:" -A 3 error_log N=$(tail -n 1 error_log | wc -c) # " Command: " and "^\n" => 13 diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc index ef1e9bafb098..eb0f4ab4e070 100644 --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc @@ -91,7 +91,9 @@ esac if grep -q "Create/append/" README && grep -q "imm-value" README; then echo 'p:kprobes/testevent _do_fork' > kprobe_events check_error '^r:kprobes/testevent do_exit' # DIFF_PROBE_TYPE -echo 'p:kprobes/testevent _do_fork abcd=\1' > kprobe_events + +# Explicitly use printf "%s" to not interpret \1 +printf "%s" 'p:kprobes/testevent _do_fork abcd=\1' > kprobe_events check_error 'p:kprobes/testevent _do_fork ^bcd=\1' # DIFF_ARG_TYPE check_error 'p:kprobes/testevent _do_fork ^abcd=\1:u8' # DIFF_ARG_TYPE check_error 'p:kprobes/testevent _do_fork ^abcd=\"foo"' # DIFF_ARG_TYPE
Since the built-in echo has different behavior in POSIX shell (dash) and bash, kprobe_syntax_errors.tc can fail on dash which interpret backslash escape automatically. To fix this issue, we explicitly use printf "%s" (not interpret backslash escapes) if the command string can include backslash. Reported-by: Liu Yiding <yidingx.liu@intel.com> Suggested-by: Xiao Yang <yangx.jy@cn.fujitsu.com> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> --- tools/testing/selftests/ftrace/test.d/functions | 8 +++++--- .../ftrace/test.d/kprobe/kprobe_syntax_errors.tc | 4 +++- 2 files changed, 8 insertions(+), 4 deletions(-)