Re: [PATCH v2] selftests/ftrace: Use printf for backslash included command
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 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
Suggested-by: Xiao Yang
Signed-off-by: Masami Hiramatsu
---
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
Re: [PATCH v2] selftests/ftrace: Use printf for backslash included command
Hi Shuah,
Could you pick this to kselftest-next?
Thank you,
On Mon, 11 May 2020 22:36:27 +0900
Masami Hiramatsu 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
> Suggested-by: Xiao Yang
> Signed-off-by: Masami Hiramatsu
> ---
> 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
>
--
Masami Hiramatsu
Re: [PATCH v2] selftests/ftrace: Use printf for backslash included command
On Mon, 11 May 2020 14:59:20 + David Laight 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
RE: [PATCH v2] selftests/ftrace: Use printf for backslash included command
> > +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)
Re: [PATCH v2] selftests/ftrace: Use printf for backslash included command
On Mon, 11 May 2020 13:46:35 + David Laight 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! -- Masami Hiramatsu
Re: [PATCH v2] selftests/ftrace: Use printf for backslash included command
On Mon, 11 May 2020 15:42:10 +0200 Andreas Schwab 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, -- Masami Hiramatsu
RE: [PATCH v2] selftests/ftrace: Use printf for backslash included command
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)
Re: [PATCH v2] selftests/ftrace: Use printf for backslash included command
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. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different."
Re: [PATCH v2] selftests/ftrace: Use printf for backslash included command
Hi Andreas and David,
OK, what about this fix?
On Mon, 11 May 2020 22:36:27 +0900
Masami Hiramatsu 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
> Suggested-by: Xiao Yang
> Signed-off-by: Masami Hiramatsu
> ---
> 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
>
--
Masami Hiramatsu
[PATCH v2] selftests/ftrace: Use printf for backslash included command
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
Suggested-by: Xiao Yang
Signed-off-by: Masami Hiramatsu
---
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
