2019-02-20 16:14:33

by Juerg Haefliger

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

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 <[email protected]>
---
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

2019-02-20 19:51:54

by Steven Rostedt

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

On Wed, 20 Feb 2019 17:13:33 +0100
Juerg Haefliger <[email protected]> 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 <[email protected]>
> ---
> 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

2019-02-20 20:21:54

by Juerg Haefliger

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

On Wed, 20 Feb 2019 14:49:34 -0500
Steven Rostedt <[email protected]> wrote:

> On Wed, 20 Feb 2019 17:13:33 +0100
> Juerg Haefliger <[email protected]> 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 <[email protected]>
> > ---
> > 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

Attachments:

2019-02-20 20:33:55

by Adam Borowski

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

On Wed, Feb 20, 2019 at 09:20:20PM +0100, Juerg Haefliger wrote:
> On Wed, 20 Feb 2019 14:49:34 -0500
> Steven Rostedt <[email protected]> wrote:
>
> > On Wed, 20 Feb 2019 17:13:33 +0100
> > Juerg Haefliger <[email protected]> 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?
⠈⠳⣄⠀⠀⠀⠀

2019-02-22 00:30:47

by Masami Hiramatsu

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

Hi Juerg,

On Wed, 20 Feb 2019 17:13:33 +0100
Juerg Haefliger <[email protected]> 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 <[email protected]>

2019-02-22 09:11:22

by Juerg Haefliger

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

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

2019-02-22 09:11:34

by Juerg Haefliger

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

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 <[email protected]>
---
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

2019-02-22 09:11:56

by Juerg Haefliger

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

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

Signed-off-by: Juerg Haefliger <[email protected]>
---
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

2019-02-22 15:00:06

by Masami Hiramatsu

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

On Fri, 22 Feb 2019 10:10:21 +0100
Juerg Haefliger <[email protected]> wrote:

> The \e sequence character is not POSIX. Fix that by using \033 instead.
>
> Signed-off-by: Juerg Haefliger <[email protected]>

This also looks good to me.

Acked-by: Masami Hiramatsu <[email protected]>

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 <[email protected]>

2019-02-22 15:00:24

by Masami Hiramatsu

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

On Fri, 22 Feb 2019 10:10:20 +0100
Juerg Haefliger <[email protected]> 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 <[email protected]>

Looks good to me.

Acked-by: Masami Hiramatsu <[email protected]>

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 <[email protected]>

2019-02-22 16:00:18

by Shuah Khan

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

Hi Juerg,

On 2/22/19 7:58 AM, Masami Hiramatsu wrote:
> On Fri, 22 Feb 2019 10:10:20 +0100
> Juerg Haefliger <[email protected]> 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 <[email protected]>
>
> Looks good to me.
>
> Acked-by: Masami Hiramatsu <[email protected]>
>

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

2019-02-22 16:00:28

by Shuah Khan

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

On 2/22/19 7:59 AM, Masami Hiramatsu wrote:
> On Fri, 22 Feb 2019 10:10:21 +0100
> Juerg Haefliger <[email protected]> wrote:
>
>> The \e sequence character is not POSIX. Fix that by using \033 instead.
>>
>> Signed-off-by: Juerg Haefliger <[email protected]>
>
> This also looks good to me.
>
> Acked-by: Masami Hiramatsu <[email protected]>
>
> 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

2019-02-22 19:04:44

by Steven Rostedt

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

On Fri, 22 Feb 2019 08:59:42 -0700
shuah <[email protected]> wrote:

> On 2/22/19 7:59 AM, Masami Hiramatsu wrote:
> > On Fri, 22 Feb 2019 10:10:21 +0100
> > Juerg Haefliger <[email protected]> wrote:
> >
> >> The \e sequence character is not POSIX. Fix that by using \033 instead.
> >>
> >> Signed-off-by: Juerg Haefliger <[email protected]>
> >
> > This also looks good to me.
> >
> > Acked-by: Masami Hiramatsu <[email protected]>
> >
> > 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) <[email protected]>

-- Steve

2019-02-22 20:53:36

by Juerg Haefliger

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

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) <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
Signed-off-by: Juerg Haefliger <[email protected]>
---
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

2019-02-22 20:53:56

by Juerg Haefliger

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

Add [email protected] and [email protected].

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

2019-02-22 20:54:32

by Juerg Haefliger

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

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) <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
Signed-off-by: Juerg Haefliger <[email protected]>
---
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

2019-02-22 20:54:50

by Juerg Haefliger

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

Add [email protected] and [email protected].

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

2019-02-22 20:55:44

by Juerg Haefliger

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

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

Acked-by: Steven Rostedt (VMware) <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
Signed-off-by: Juerg Haefliger <[email protected]>
---
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

2019-02-22 20:55:46

by Juerg Haefliger

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

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) <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
Signed-off-by: Juerg Haefliger <[email protected]>
---
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

2019-02-22 20:55:52

by Juerg Haefliger

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

Add [email protected] and [email protected].

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

2019-02-22 22:34:29

by Shuah Khan

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

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) <[email protected]>
> Acked-by: Masami Hiramatsu <[email protected]>
> Signed-off-by: Juerg Haefliger <[email protected]>
> ---
> 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

2019-02-22 22:47:10

by Shuah Khan

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

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) <[email protected]>
> Acked-by: Masami Hiramatsu <[email protected]>
> Signed-off-by: Juerg Haefliger <[email protected]>
> ---
> 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

2019-02-23 12:27:26

by Juerg Haefliger

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

On Fri, 22 Feb 2019 15:46:03 -0700
shuah <[email protected]> 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) <[email protected]>
> > Acked-by: Masami Hiramatsu <[email protected]>
> > Signed-off-by: Juerg Haefliger <[email protected]>
> > ---
> > 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/[email protected]/T/#u
Sorry about not copying you on that patch. Want me to resend?

...Juerg


> thanks,
> -- Shuah

Attachments:

2019-02-25 13:01:50

by Shuah Khan

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

On 2/23/19 5:25 AM, Juerg Haefliger wrote:
> On Fri, 22 Feb 2019 15:46:03 -0700
> shuah <[email protected]> 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) <[email protected]>
>>> Acked-by: Masami Hiramatsu <[email protected]>
>>> Signed-off-by: Juerg Haefliger <[email protected]>
>>> ---
>>> 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/[email protected]/T/#u
> Sorry about not copying you on that patch. Want me to resend?
>
> ...Juerg
>

Yes. Please.

thanks,
-- Shuah