[PATCH] selftests/ftrace: Make the coloring POSIX compliant

[PATCH] selftests/ftrace: Make the coloring POSIX compliant

[Juerg Haefliger]

echo -e and \e are not POSIX. Depending on what /bin/sh is, we can get
incorrect output like:
$ -e -n [1] Basic trace file check
$ -e 	[PASS]

Fix that by using \033 instead of \e and printf.

Signed-off-by: Juerg Haefliger <juergh@canonical.com>
---
 tools/testing/selftests/ftrace/ftracetest | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
index fc755e1b50f1..f200898e3e2c 100755
--- a/tools/testing/selftests/ftrace/ftracetest
+++ b/tools/testing/selftests/ftrace/ftracetest
@@ -161,10 +161,10 @@ color_green=
 color_blue=
 # If stdout exists and number of colors is eight or more, use them
 if [ -t 1 -a "$ncolors" -ge 8 ]; then
-  color_reset="\e[0m"
-  color_red="\e[31m"
-  color_green="\e[32m"
-  color_blue="\e[34m"
+  color_reset="\033[0m"
+  color_red="\033[31m"
+  color_green="\033[32m"
+  color_blue="\033[34m"
 fi
 
 strip_esc() {
@@ -173,8 +173,13 @@ strip_esc() {
 }
 
 prlog() { # messages
-  echo -e "$@"
-  [ "$LOG_FILE" ] && echo -e "$@" | strip_esc >> $LOG_FILE
+  newline="\n"
+  if [ "$1" = "-n" ] ; then
+    newline=
+    shift
+  fi
+  printf "$@$newline"
+  [ "$LOG_FILE" ] && printf "$@$newline" | strip_esc >> $LOG_FILE
 }
 catlog() { #file
   cat $1
-- 
2.19.1

Re: [PATCH] selftests/ftrace: Make the coloring POSIX compliant

[Steven Rostedt]

On Wed, 20 Feb 2019 17:13:33 +0100
Juerg Haefliger <juerg.haefliger@canonical.com> wrote:

> echo -e and \e are not POSIX. Depending on what /bin/sh is, we can get
> incorrect output like:

I'm curious to which shell this is.

> $ -e -n [1] Basic trace file check
> $ -e 	[PASS]
> 
> Fix that by using \033 instead of \e and printf.
> 
> Signed-off-by: Juerg Haefliger <juergh@canonical.com>
> ---
>  tools/testing/selftests/ftrace/ftracetest | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
> index fc755e1b50f1..f200898e3e2c 100755
> --- a/tools/testing/selftests/ftrace/ftracetest
> +++ b/tools/testing/selftests/ftrace/ftracetest
> @@ -161,10 +161,10 @@ color_green=
>  color_blue=
>  # If stdout exists and number of colors is eight or more, use them
>  if [ -t 1 -a "$ncolors" -ge 8 ]; then
> -  color_reset="\e[0m"
> -  color_red="\e[31m"
> -  color_green="\e[32m"
> -  color_blue="\e[34m"
> +  color_reset="\033[0m"
> +  color_red="\033[31m"
> +  color_green="\033[32m"
> +  color_blue="\033[34m"
>  fi
>  
>  strip_esc() {
> @@ -173,8 +173,13 @@ strip_esc() {
>  }
>  
>  prlog() { # messages
> -  echo -e "$@"
> -  [ "$LOG_FILE" ] && echo -e "$@" | strip_esc >> $LOG_FILE
> +  newline="\n"
> +  if [ "$1" = "-n" ] ; then
> +    newline=
> +    shift
> +  fi
> +  printf "$@$newline"
> +  [ "$LOG_FILE" ] && printf "$@$newline" | strip_esc >> $LOG_FILE
>  }
>  catlog() { #file
>    cat $1

This should probably be split into two patches, as they are two
different issues.

-- Steve

Re: [PATCH] selftests/ftrace: Make the coloring POSIX compliant

[Juerg Haefliger]

[-- Attachment #1: Type: text/plain, Size: 2019 bytes --]

On Wed, 20 Feb 2019 14:49:34 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed, 20 Feb 2019 17:13:33 +0100
> Juerg Haefliger <juerg.haefliger@canonical.com> wrote:
> 
> > echo -e and \e are not POSIX. Depending on what /bin/sh is, we can get
> > incorrect output like:  
> 
> I'm curious to which shell this is.

Quite frankly I don't know but that's the output we get when we run it in
Jenkins. I'll try to find out.


> > $ -e -n [1] Basic trace file check
> > $ -e 	[PASS]
> > 
> > Fix that by using \033 instead of \e and printf.
> > 
> > Signed-off-by: Juerg Haefliger <juergh@canonical.com>
> > ---
> >  tools/testing/selftests/ftrace/ftracetest | 17 +++++++++++------
> >  1 file changed, 11 insertions(+), 6 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
> > index fc755e1b50f1..f200898e3e2c 100755
> > --- a/tools/testing/selftests/ftrace/ftracetest
> > +++ b/tools/testing/selftests/ftrace/ftracetest
> > @@ -161,10 +161,10 @@ color_green=
> >  color_blue=
> >  # If stdout exists and number of colors is eight or more, use them
> >  if [ -t 1 -a "$ncolors" -ge 8 ]; then
> > -  color_reset="\e[0m"
> > -  color_red="\e[31m"
> > -  color_green="\e[32m"
> > -  color_blue="\e[34m"
> > +  color_reset="\033[0m"
> > +  color_red="\033[31m"
> > +  color_green="\033[32m"
> > +  color_blue="\033[34m"
> >  fi
> >  
> >  strip_esc() {
> > @@ -173,8 +173,13 @@ strip_esc() {
> >  }
> >  
> >  prlog() { # messages
> > -  echo -e "$@"
> > -  [ "$LOG_FILE" ] && echo -e "$@" | strip_esc >> $LOG_FILE
> > +  newline="\n"
> > +  if [ "$1" = "-n" ] ; then
> > +    newline=
> > +    shift
> > +  fi
> > +  printf "$@$newline"
> > +  [ "$LOG_FILE" ] && printf "$@$newline" | strip_esc >> $LOG_FILE
> >  }
> >  catlog() { #file
> >    cat $1  
> 
> This should probably be split into two patches, as they are two
> different issues.

Will do.

...Juerg

 
> -- Steve


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] selftests/ftrace: Make the coloring POSIX compliant

[Adam Borowski]

On Wed, Feb 20, 2019 at 09:20:20PM +0100, Juerg Haefliger wrote:
> On Wed, 20 Feb 2019 14:49:34 -0500
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > On Wed, 20 Feb 2019 17:13:33 +0100
> > Juerg Haefliger <juerg.haefliger@canonical.com> wrote:
> > 
> > > echo -e and \e are not POSIX. Depending on what /bin/sh is, we can get
> > > incorrect output like:  
> > 
> > I'm curious to which shell this is.
> 
> Quite frankly I don't know but that's the output we get when we run it in
> Jenkins. I'll try to find out.

The only shell that did not support \e was dash -- I fixed it myself on
2017-01-24; thus I expect any Ubuntus prior to Zesty to require \033.  This
means your Jenkins likely runs Xenial.

\e has been supported since ages in at least: bash zsh mksh sash posh ksh
busybox:sh; also in perl python ruby lua php, gcc clang tcc -- MSVC being
the only other exception I know about.

Indeed POSIX doesn't specify \e, but as it pretends there are charsets other
than ASCII and EBCDIC, it can't.  There's no escape in its "portable
character set".


Meow!
-- 
⢀⣴⠾⠻⢶⣦⠀
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Have you accepted Khorne as your lord and saviour?
⠈⠳⣄⠀⠀⠀⠀

Re: [PATCH] selftests/ftrace: Make the coloring POSIX compliant

[Masami Hiramatsu]

Hi Juerg,

On Wed, 20 Feb 2019 17:13:33 +0100
Juerg Haefliger <juerg.haefliger@canonical.com> wrote:

> echo -e and \e are not POSIX. Depending on what /bin/sh is, we can get
> incorrect output like:
> $ -e -n [1] Basic trace file check
> $ -e 	[PASS]
> 
> Fix that by using \033 instead of \e and printf.

OK, as far as I can check with checkbashisms, echo -e is not acceptable.

$ checkbashisms ./ftracetest 
possible bashism in ./ftracetest line 176 (echo -e):
  echo -e "$@"
possible bashism in ./ftracetest line 177 (echo -e):
  [ "$LOG_FILE" ] && echo -e "$@" | strip_esc >> $LOG_FILE

So it should be fixed, even other shells support it.
(Or, update checkbashisms command...)

However,

>  prlog() { # messages
> -  echo -e "$@"
> -  [ "$LOG_FILE" ] && echo -e "$@" | strip_esc >> $LOG_FILE
> +  newline="\n"
> +  if [ "$1" = "-n" ] ; then
> +    newline=
> +    shift
> +  fi
> +  printf "$@$newline"
> +  [ "$LOG_FILE" ] && printf "$@$newline" | strip_esc >> $LOG_FILE

This doesn't work when prlog gets several arguments. (like the summary line)
for example I got below result.

# of passed: # of failed: # of unresolved: # of untested: # of unsupported: # of xfailed: # of undefined(test bug): 

Replacing $@ with $* shows correct result. Could you fix it?

Thank you,


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[PATCH v2 0/2] selftests/ftrace: Make ftracetest POSIX compliant

[Juerg Haefliger]

The recent addition of colored output introduced some non-POSIX-compliant
constructs. Fix that.

Juerg Haefliger (2):
  selftests/ftrace: Replace echo -e with printf
  selftests/ftrace: Replace \e with \033

 tools/testing/selftests/ftrace/ftracetest | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

-- 
2.19.1

[PATCH v2 1/2] selftests/ftrace: Replace echo -e with printf

[Juerg Haefliger]

echo -e is not POSIX. Depending on what /bin/sh is, we can get
incorrect output like:
$ -e -n [1] Basic trace file check
$ -e 	[PASS]

Fix that by using printf instead.

Signed-off-by: Juerg Haefliger <juergh@canonical.com>
---
 tools/testing/selftests/ftrace/ftracetest | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
index fc755e1b50f1..20c9c0ad8682 100755
--- a/tools/testing/selftests/ftrace/ftracetest
+++ b/tools/testing/selftests/ftrace/ftracetest
@@ -173,8 +173,13 @@ strip_esc() {
 }
 
 prlog() { # messages
-  echo -e "$@"
-  [ "$LOG_FILE" ] && echo -e "$@" | strip_esc >> $LOG_FILE
+  newline="\n"
+  if [ "$1" = "-n" ] ; then
+    newline=
+    shift
+  fi
+  printf "$*$newline"
+  [ "$LOG_FILE" ] && printf "$*$newline" | strip_esc >> $LOG_FILE
 }
 catlog() { #file
   cat $1
-- 
2.19.1

[PATCH v2 2/2] selftests/ftrace: Replace \e with \033

[Juerg Haefliger]

The \e sequence character is not POSIX. Fix that by using \033 instead.

Signed-off-by: Juerg Haefliger <juergh@canonical.com>
---
 tools/testing/selftests/ftrace/ftracetest | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
index 20c9c0ad8682..136387422b00 100755
--- a/tools/testing/selftests/ftrace/ftracetest
+++ b/tools/testing/selftests/ftrace/ftracetest
@@ -161,10 +161,10 @@ color_green=
 color_blue=
 # If stdout exists and number of colors is eight or more, use them
 if [ -t 1 -a "$ncolors" -ge 8 ]; then
-  color_reset="\e[0m"
-  color_red="\e[31m"
-  color_green="\e[32m"
-  color_blue="\e[34m"
+  color_reset="\033[0m"
+  color_red="\033[31m"
+  color_green="\033[32m"
+  color_blue="\033[34m"
 fi
 
 strip_esc() {
-- 
2.19.1

Re: [PATCH v2 1/2] selftests/ftrace: Replace echo -e with printf

[Masami Hiramatsu]

On Fri, 22 Feb 2019 10:10:20 +0100
Juerg Haefliger <juerg.haefliger@canonical.com> wrote:

> echo -e is not POSIX. Depending on what /bin/sh is, we can get
> incorrect output like:
> $ -e -n [1] Basic trace file check
> $ -e 	[PASS]
> 
> Fix that by using printf instead.
> 
> Signed-off-by: Juerg Haefliger <juergh@canonical.com>

Looks good to me.

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Thanks!

> ---
>  tools/testing/selftests/ftrace/ftracetest | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
> index fc755e1b50f1..20c9c0ad8682 100755
> --- a/tools/testing/selftests/ftrace/ftracetest
> +++ b/tools/testing/selftests/ftrace/ftracetest
> @@ -173,8 +173,13 @@ strip_esc() {
>  }
>  
>  prlog() { # messages
> -  echo -e "$@"
> -  [ "$LOG_FILE" ] && echo -e "$@" | strip_esc >> $LOG_FILE
> +  newline="\n"
> +  if [ "$1" = "-n" ] ; then
> +    newline=
> +    shift
> +  fi
> +  printf "$*$newline"
> +  [ "$LOG_FILE" ] && printf "$*$newline" | strip_esc >> $LOG_FILE
>  }
>  catlog() { #file
>    cat $1
> -- 
> 2.19.1
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH v2 2/2] selftests/ftrace: Replace \e with \033

[Masami Hiramatsu]

On Fri, 22 Feb 2019 10:10:21 +0100
Juerg Haefliger <juerg.haefliger@canonical.com> wrote:

> The \e sequence character is not POSIX. Fix that by using \033 instead.
> 
> Signed-off-by: Juerg Haefliger <juergh@canonical.com>

This also looks good to me.

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Thank you!

> ---
>  tools/testing/selftests/ftrace/ftracetest | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
> index 20c9c0ad8682..136387422b00 100755
> --- a/tools/testing/selftests/ftrace/ftracetest
> +++ b/tools/testing/selftests/ftrace/ftracetest
> @@ -161,10 +161,10 @@ color_green=
>  color_blue=
>  # If stdout exists and number of colors is eight or more, use them
>  if [ -t 1 -a "$ncolors" -ge 8 ]; then
> -  color_reset="\e[0m"
> -  color_red="\e[31m"
> -  color_green="\e[32m"
> -  color_blue="\e[34m"
> +  color_reset="\033[0m"
> +  color_red="\033[31m"
> +  color_green="\033[32m"
> +  color_blue="\033[34m"
>  fi
>  
>  strip_esc() {
> -- 
> 2.19.1
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH v2 1/2] selftests/ftrace: Replace echo -e with printf

[shuah]

Hi Juerg,

On 2/22/19 7:58 AM, Masami Hiramatsu wrote:
> On Fri, 22 Feb 2019 10:10:20 +0100
> Juerg Haefliger <juerg.haefliger@canonical.com> wrote:
> 
>> echo -e is not POSIX. Depending on what /bin/sh is, we can get
>> incorrect output like:
>> $ -e -n [1] Basic trace file check
>> $ -e 	[PASS]
>>
>> Fix that by using printf instead.
>>
>> Signed-off-by: Juerg Haefliger <juergh@canonical.com>
> 
> Looks good to me.
> 
> Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> 

I wasn't on the To or Cc on the original patch. Make sure to
run the get_maintainers and include the people it recommends.

Please resend the patch to me, so I can take it for 5.1-rc1

thanks,
-- Shuah

Re: [PATCH v2 2/2] selftests/ftrace: Replace \e with \033

[shuah]

On 2/22/19 7:59 AM, Masami Hiramatsu wrote:
> On Fri, 22 Feb 2019 10:10:21 +0100
> Juerg Haefliger <juerg.haefliger@canonical.com> wrote:
> 
>> The \e sequence character is not POSIX. Fix that by using \033 instead.
>>
>> Signed-off-by: Juerg Haefliger <juergh@canonical.com>
> 
> This also looks good to me.
> 
> Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> 
> Thank you!
> 


I wasn't on the To or Cc on the original patch. Make sure to
run the get_maintainers and include the people it recommends.

Please resend the patch to me, so I can take it for 5.1-rc1

thanks,
-- Shuah

Re: [PATCH v2 2/2] selftests/ftrace: Replace \e with \033

[Steven Rostedt]

On Fri, 22 Feb 2019 08:59:42 -0700
shuah <shuah@kernel.org> wrote:

> On 2/22/19 7:59 AM, Masami Hiramatsu wrote:
> > On Fri, 22 Feb 2019 10:10:21 +0100
> > Juerg Haefliger <juerg.haefliger@canonical.com> wrote:
> >   
> >> The \e sequence character is not POSIX. Fix that by using \033 instead.
> >>
> >> Signed-off-by: Juerg Haefliger <juergh@canonical.com>  
> > 
> > This also looks good to me.
> > 
> > Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> > 
> > Thank you!
> >   
> 
> 
> I wasn't on the To or Cc on the original patch. Make sure to
> run the get_maintainers and include the people it recommends.
> 
> Please resend the patch to me, so I can take it for 5.1-rc1


And add both Masami and my Acked-by when you send it to Shuah (for both
patches).

Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve

[RESEND PATCH v2 0/2] selftests/ftrace: Make ftracetest POSIX compliant

[Juerg Haefliger]

Add shuah@kernel.org and linux-kselftest@vger.kernel.org.

The recent addition of colored output introduced some non-POSIX-compliant
constructs. Fix that.

Juerg Haefliger (2):
  selftests/ftrace: Replace echo -e with printf
  selftests/ftrace: Replace \e with \033

 tools/testing/selftests/ftrace/ftracetest | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

-- 
2.19.1

[RESEND PATCH v2 1/2] selftests/ftrace: Replace echo -e with printf

[Juerg Haefliger]

echo -e is not POSIX. Depending on what /bin/sh is, we can get
incorrect output like:
$ -e -n [1] Basic trace file check
$ -e 	[PASS]

Fix that by using printf instead.

Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Juerg Haefliger <juergh@canonical.com>
---
 tools/testing/selftests/ftrace/ftracetest | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
index fc755e1b50f1..20c9c0ad8682 100755
--- a/tools/testing/selftests/ftrace/ftracetest
+++ b/tools/testing/selftests/ftrace/ftracetest
@@ -173,8 +173,13 @@ strip_esc() {
 }
 
 prlog() { # messages
-  echo -e "$@"
-  [ "$LOG_FILE" ] && echo -e "$@" | strip_esc >> $LOG_FILE
+  newline="\n"
+  if [ "$1" = "-n" ] ; then
+    newline=
+    shift
+  fi
+  printf "$*$newline"
+  [ "$LOG_FILE" ] && printf "$*$newline" | strip_esc >> $LOG_FILE
 }
 catlog() { #file
   cat $1
-- 
2.19.1

[RESEND PATCH v2 0/2] selftests/ftrace: Make ftracetest POSIX compliant

[Juerg Haefliger]

Add shuah@kernel.org and linux-kselftest@vger.kernel.org.

The recent addition of colored output introduced some non-POSIX-compliant
constructs. Fix that.

Juerg Haefliger (2):
  selftests/ftrace: Replace echo -e with printf
  selftests/ftrace: Replace \e with \033

 tools/testing/selftests/ftrace/ftracetest | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

-- 
2.19.1

[RESEND PATCH v2 1/2] selftests/ftrace: Replace echo -e with printf

[Juerg Haefliger]

echo -e is not POSIX. Depending on what /bin/sh is, we can get
incorrect output like:
$ -e -n [1] Basic trace file check
$ -e 	[PASS]

Fix that by using printf instead.

Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Juerg Haefliger <juergh@canonical.com>
---
 tools/testing/selftests/ftrace/ftracetest | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
index fc755e1b50f1..20c9c0ad8682 100755
--- a/tools/testing/selftests/ftrace/ftracetest
+++ b/tools/testing/selftests/ftrace/ftracetest
@@ -173,8 +173,13 @@ strip_esc() {
 }
 
 prlog() { # messages
-  echo -e "$@"
-  [ "$LOG_FILE" ] && echo -e "$@" | strip_esc >> $LOG_FILE
+  newline="\n"
+  if [ "$1" = "-n" ] ; then
+    newline=
+    shift
+  fi
+  printf "$*$newline"
+  [ "$LOG_FILE" ] && printf "$*$newline" | strip_esc >> $LOG_FILE
 }
 catlog() { #file
   cat $1
-- 
2.19.1

[RESEND PATCH v2 0/2] selftests/ftrace: Make ftracetest POSIX compliant

[Juerg Haefliger]

Add shuah@kernel.org and linux-kselftest@vger.kernel.org.

The recent addition of colored output introduced some non-POSIX-compliant
constructs. Fix that.

Juerg Haefliger (2):
  selftests/ftrace: Replace echo -e with printf
  selftests/ftrace: Replace \e with \033

 tools/testing/selftests/ftrace/ftracetest | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

-- 
2.19.1

[RESEND PATCH v2 1/2] selftests/ftrace: Replace echo -e with printf

[Juerg Haefliger]

echo -e is not POSIX. Depending on what /bin/sh is, we can get
incorrect output like:
$ -e -n [1] Basic trace file check
$ -e 	[PASS]

Fix that by using printf instead.

Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Juerg Haefliger <juergh@canonical.com>
---
 tools/testing/selftests/ftrace/ftracetest | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
index fc755e1b50f1..20c9c0ad8682 100755
--- a/tools/testing/selftests/ftrace/ftracetest
+++ b/tools/testing/selftests/ftrace/ftracetest
@@ -173,8 +173,13 @@ strip_esc() {
 }
 
 prlog() { # messages
-  echo -e "$@"
-  [ "$LOG_FILE" ] && echo -e "$@" | strip_esc >> $LOG_FILE
+  newline="\n"
+  if [ "$1" = "-n" ] ; then
+    newline=
+    shift
+  fi
+  printf "$*$newline"
+  [ "$LOG_FILE" ] && printf "$*$newline" | strip_esc >> $LOG_FILE
 }
 catlog() { #file
   cat $1
-- 
2.19.1

[RESEND PATCH v2 2/2] selftests/ftrace: Replace \e with \033

[Juerg Haefliger]

The \e sequence character is not POSIX. Fix that by using \033 instead.

Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Juerg Haefliger <juergh@canonical.com>
---
 tools/testing/selftests/ftrace/ftracetest | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
index 20c9c0ad8682..136387422b00 100755
--- a/tools/testing/selftests/ftrace/ftracetest
+++ b/tools/testing/selftests/ftrace/ftracetest
@@ -161,10 +161,10 @@ color_green=
 color_blue=
 # If stdout exists and number of colors is eight or more, use them
 if [ -t 1 -a "$ncolors" -ge 8 ]; then
-  color_reset="\e[0m"
-  color_red="\e[31m"
-  color_green="\e[32m"
-  color_blue="\e[34m"
+  color_reset="\033[0m"
+  color_red="\033[31m"
+  color_green="\033[32m"
+  color_blue="\033[34m"
 fi
 
 strip_esc() {
-- 
2.19.1

Re: [RESEND PATCH v2 1/2] selftests/ftrace: Replace echo -e with printf

[shuah]

On 2/22/19 1:53 PM, Juerg Haefliger wrote:
> echo -e is not POSIX. Depending on what /bin/sh is, we can get
> incorrect output like:
> $ -e -n [1] Basic trace file check
> $ -e 	[PASS]
> 
> Fix that by using printf instead.
> 
> Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Juerg Haefliger <juergh@canonical.com>
> ---
>   tools/testing/selftests/ftrace/ftracetest | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
> index fc755e1b50f1..20c9c0ad8682 100755
> --- a/tools/testing/selftests/ftrace/ftracetest
> +++ b/tools/testing/selftests/ftrace/ftracetest
> @@ -173,8 +173,13 @@ strip_esc() {
>   }
>   
>   prlog() { # messages
> -  echo -e "$@"
> -  [ "$LOG_FILE" ] && echo -e "$@" | strip_esc >> $LOG_FILE
> +  newline="\n"
> +  if [ "$1" = "-n" ] ; then
> +    newline=
> +    shift
> +  fi
> +  printf "$*$newline"
> +  [ "$LOG_FILE" ] && printf "$*$newline" | strip_esc >> $LOG_FILE
>   }
>   catlog() { #file
>     cat $1
> 

This patch applied to linux-kselftest next which is at 5.0rc6
I queued this for 5.1-rc1.

thanks,
-- Shuah

Re: [RESEND PATCH v2 2/2] selftests/ftrace: Replace \e with \033

[shuah]

On 2/22/19 1:53 PM, Juerg Haefliger wrote:
> The \e sequence character is not POSIX. Fix that by using \033 instead.
> 
> Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Juerg Haefliger <juergh@canonical.com>
> ---
>   tools/testing/selftests/ftrace/ftracetest | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
> index 20c9c0ad8682..136387422b00 100755
> --- a/tools/testing/selftests/ftrace/ftracetest
> +++ b/tools/testing/selftests/ftrace/ftracetest
> @@ -161,10 +161,10 @@ color_green=
>   color_blue=
>   # If stdout exists and number of colors is eight or more, use them
>   if [ -t 1 -a "$ncolors" -ge 8 ]; then
> -  color_reset="\e[0m"
> -  color_red="\e[31m"
> -  color_green="\e[32m"
> -  color_blue="\e[34m"
> +  color_reset="\033[0m"
> +  color_red="\033[31m"
> +  color_green="\033[32m"
> +  color_blue="\033[34m"
>   fi
>   
>   strip_esc() {
> 

Which release is this patch based on?

This one failed to apply to linux-kselftest next - I resolved a minor
merge conflict and applied it. Please review to make sure it looks good.

thanks,
-- Shuah

Re: [RESEND PATCH v2 2/2] selftests/ftrace: Replace \e with \033

[Juerg Haefliger]

[-- Attachment #1: Type: text/plain, Size: 1661 bytes --]

On Fri, 22 Feb 2019 15:46:03 -0700
shuah <shuah@kernel.org> wrote:

> On 2/22/19 1:53 PM, Juerg Haefliger wrote:
> > The \e sequence character is not POSIX. Fix that by using \033 instead.
> > 
> > Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> > Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> > Signed-off-by: Juerg Haefliger <juergh@canonical.com>
> > ---
> >   tools/testing/selftests/ftrace/ftracetest | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
> > index 20c9c0ad8682..136387422b00 100755
> > --- a/tools/testing/selftests/ftrace/ftracetest
> > +++ b/tools/testing/selftests/ftrace/ftracetest
> > @@ -161,10 +161,10 @@ color_green=
> >   color_blue=
> >   # If stdout exists and number of colors is eight or more, use them
> >   if [ -t 1 -a "$ncolors" -ge 8 ]; then
> > -  color_reset="\e[0m"
> > -  color_red="\e[31m"
> > -  color_green="\e[32m"
> > -  color_blue="\e[34m"
> > +  color_reset="\033[0m"
> > +  color_red="\033[31m"
> > +  color_green="\033[32m"
> > +  color_blue="\033[34m"
> >   fi
> >   
> >   strip_esc() {
> >   
> 
> Which release is this patch based on?
> 
> This one failed to apply to linux-kselftest next - I resolved a minor
> merge conflict and applied it. Please review to make sure it looks good.

Looks good. It didn't apply cleanly because you don't have
https://lore.kernel.org/lkml/20190220153706.24686-1-juergh@canonical.com/T/#u
Sorry about not copying you on that patch. Want me to resend?

...Juerg


> thanks,
> -- Shuah


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RESEND PATCH v2 2/2] selftests/ftrace: Replace \e with \033

[shuah]

On 2/23/19 5:25 AM, Juerg Haefliger wrote:
> On Fri, 22 Feb 2019 15:46:03 -0700
> shuah <shuah@kernel.org> wrote:
> 
>> On 2/22/19 1:53 PM, Juerg Haefliger wrote:
>>> The \e sequence character is not POSIX. Fix that by using \033 instead.
>>>
>>> Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
>>> Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
>>> Signed-off-by: Juerg Haefliger <juergh@canonical.com>
>>> ---
>>>    tools/testing/selftests/ftrace/ftracetest | 8 ++++----
>>>    1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
>>> index 20c9c0ad8682..136387422b00 100755
>>> --- a/tools/testing/selftests/ftrace/ftracetest
>>> +++ b/tools/testing/selftests/ftrace/ftracetest
>>> @@ -161,10 +161,10 @@ color_green=
>>>    color_blue=
>>>    # If stdout exists and number of colors is eight or more, use them
>>>    if [ -t 1 -a "$ncolors" -ge 8 ]; then
>>> -  color_reset="\e[0m"
>>> -  color_red="\e[31m"
>>> -  color_green="\e[32m"
>>> -  color_blue="\e[34m"
>>> +  color_reset="\033[0m"
>>> +  color_red="\033[31m"
>>> +  color_green="\033[32m"
>>> +  color_blue="\033[34m"
>>>    fi
>>>    
>>>    strip_esc() {
>>>    
>>
>> Which release is this patch based on?
>>
>> This one failed to apply to linux-kselftest next - I resolved a minor
>> merge conflict and applied it. Please review to make sure it looks good.
> 
> Looks good. It didn't apply cleanly because you don't have
> https://lore.kernel.org/lkml/20190220153706.24686-1-juergh@canonical.com/T/#u
> Sorry about not copying you on that patch. Want me to resend?
> 
> ...Juerg
> 

Yes. Please.

thanks,
-- Shuah