Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 29 May 2020 15:37:04 -0500
From:   Seth Forshee <seth.forshee@canonical.com>
To:     Steven Rostedt <rostedt@goodmis.org>,
        Ingo Molnar <mingo@redhat.com>, Shuah Khan <shuah@kernel.org>
Cc:     linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] selftests/ftrace: Use printf instead of echo in
 kprobe syntax error tests
Message-ID: <20200529203704.GA57013@ubuntu-x1>
References: <20200304222009.34663-1-seth.forshee@canonical.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200304222009.34663-1-seth.forshee@canonical.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Mar 04, 2020 at 04:20:09PM -0600, Seth Forshee wrote:
> Test cases which use echo to write strings containing backslashes
> fail with some shells, as echo's treatment of backslashes in
> strings varies between shell implementations. Use printf instead,
> as it should behave consistently across different shells. This
> requires adjustments to the strings to escape \ and % characters.
> ftrace_errlog_check() must also re-escape these characters after
> processing them to remove ^ characters.
> 
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>

Ping. Someone just asked me about this patch, and I noticed that it
hasn't been applied or received any feedback.

> ---
> Changes in v2:
>  - Escape backslashes for a couple of additional tests
> 
>  .../testing/selftests/ftrace/test.d/functions |  6 +++---
>  .../test.d/kprobe/kprobe_syntax_errors.tc     | 20 +++++++++----------
>  2 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/functions b/tools/testing/selftests/ftrace/test.d/functions
> index 5d4550591ff9..b38c6eb029e8 100644
> --- a/tools/testing/selftests/ftrace/test.d/functions
> +++ b/tools/testing/selftests/ftrace/test.d/functions
> @@ -114,11 +114,11 @@ yield() {
>  }
>  
>  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 "${2%^*}" | wc -c) # error position
> +    command=$(printf "$2" | sed -e 's/\^//g' -e 's/%/%%/g' -e 's/\\/\\\\/g')
>      echo "Test command: $command"
>      echo > error_log
> -    (! echo "$command" >> "$3" ) 2> /dev/null
> +    (! printf "$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..039c03d230b9 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
> @@ -37,14 +37,14 @@ fi
>  
>  check_error 'p vfs_read ^$none_var'	# BAD_VAR
>  
> -check_error 'p vfs_read ^%none_reg'	# BAD_REG_NAME
> +check_error 'p vfs_read ^%%none_reg'	# BAD_REG_NAME
>  check_error 'p vfs_read ^@12345678abcde'	# BAD_MEM_ADDR
>  check_error 'p vfs_read ^@+10'		# FILE_ON_KPROBE
>  
>  grep -q "imm-value" README && \
> -check_error 'p vfs_read arg1=\^x'	# BAD_IMM
> +check_error 'p vfs_read arg1=\\^x'	# BAD_IMM
>  grep -q "imm-string" README && \
> -check_error 'p vfs_read arg1=\"abcd^'	# IMMSTR_NO_CLOSE
> +check_error 'p vfs_read arg1=\\"abcd^'	# IMMSTR_NO_CLOSE
>  
>  check_error 'p vfs_read ^+0@0)'		# DEREF_NEED_BRACE
>  check_error 'p vfs_read ^+0ab1(@0)'	# BAD_DEREF_OFFS
> @@ -80,7 +80,7 @@ check_error 'p vfs_read arg1=^'			# NO_ARG_BODY
>  # instruction boundary check is valid on x86 (at this moment)
>  case $(uname -m) in
>    x86_64|i[3456]86)
> -    echo 'p vfs_read' > kprobe_events
> +    printf 'p vfs_read' > kprobe_events
>      if grep -q FTRACE ../kprobes/list ; then
>  	check_error 'p ^vfs_read+3'		# BAD_INSN_BNDRY (only if function-tracer is enabled)
>      fi
> @@ -89,13 +89,13 @@ esac
>  
>  # multiprobe errors
>  if grep -q "Create/append/" README && grep -q "imm-value" README; then
> -echo 'p:kprobes/testevent _do_fork' > kprobe_events
> +printf '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
> -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
> -check_error '^p:kprobes/testevent _do_fork abcd=\1'	# SAME_PROBE
> +printf '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
> +check_error '^p:kprobes/testevent _do_fork abcd=\\1'	# SAME_PROBE
>  fi
>  
>  exit 0
> -- 
> 2.25.0
>