On Mon, 11 May 2020 15:22:25 +0800 Xiao Yang <yangx...@cn.fujitsu.com> wrote:
> On 2020/5/7 17:15, Masami Hiramatsu wrote: > > On Thu, 7 May 2020 14:45:16 +0800 > > Xiao Yang<yangx...@cn.fujitsu.com> wrote: > > > >> On 2020/5/1 21:38, Masami Hiramatsu wrote: > >>> Since the built-in echo has different behavior in POSIX shell > >>> (dash) and bash, we forcibly use /bin/echo -E (not interpret > >>> backslash escapes) by default. > >>> > >>> This also fixes some test cases which expects built-in > >>> echo command. > >>> > >>> Reported-by: Liu Yiding<yidingx....@intel.com> > >>> Signed-off-by: Masami Hiramatsu<mhira...@kernel.org> > >>> --- > >>> tools/testing/selftests/ftrace/test.d/functions | 3 +++ > >>> .../test.d/trigger/trigger-trace-marker-hist.tc | 2 +- > >>> .../trigger-trace-marker-synthetic-kernel.tc | 4 ++++ > >>> .../trigger/trigger-trace-marker-synthetic.tc | 4 ++-- > >>> 4 files changed, 10 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/tools/testing/selftests/ftrace/test.d/functions > >>> b/tools/testing/selftests/ftrace/test.d/functions > >>> index 5d4550591ff9..ea59b6ea2c3e 100644 > >>> --- a/tools/testing/selftests/ftrace/test.d/functions > >>> +++ b/tools/testing/selftests/ftrace/test.d/functions > >>> @@ -1,3 +1,6 @@ > >>> +# Since the built-in echo has different behavior in POSIX shell (dash) > >>> and > >>> +# bash, we forcibly use /bin/echo -E (not interpret backslash escapes). > >>> +alias echo="/bin/echo -E" > >> Hi Masami, Steven > >> > >> It seems that only kprobe_syntax_errors.tc is impacted by the issue > >> currently. Is it necessary for all tests to use /bin/echo and could we > >> just make kprobe_syntax_errors.tc use /bin/echo? > > > > Yes, I would like to unify the "echo"'s behavior among the testcases > > instead of patching each failure in the future. > > Or would you have any concern on it? > Hi Masami, > > Very sorry for the late reply. > > We may not avoid fixing related failures after your change: > 1) We have to reuse built-in echo (do alias echo=echo) if we want to > test common_pid for histogram. > 2) We have to reuse built-in echo if some new tests want to interpret > backslash escapes in future. 1) yes, that's what I need to do for avoiding "pid" key histogram (but I think we should have better way to test it) 2) No, in that case you should use "/bin/echo -e" explicitly. dash's built-in echo doesn't support it. > Is it simple to provide two implementations of echo?(built-in echo and > echo command?) and then just apply echo command for kprobe_syntax_errors.tc? Hmm, OK, there might be another reason we reconsider this patch. - Alisasing echo (this patch) can avoid dash related issues but this also makes "echo" running in another process implicitly. - Using /bin/echo for backslash explicitly will be missed unless user runs it on dash, but it will keep "echo" in same process. So both have pros/cons, but your idea will be locally effected. OK, I'll retry it. Thank you, -- Masami Hiramatsu <mhira...@kernel.org>
