On Mon, 11 May 2020 15:22:25 +0800 Xiao Yang yangx.jy@cn.fujitsu.com wrote:
On 2020/5/7 17:15, Masami Hiramatsu wrote:
On Thu, 7 May 2020 14:45:16 +0800 Xiao Yangyangx.jy@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 Yidingyidingx.liu@intel.com Signed-off-by: Masami Hiramatsumhiramat@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:
- 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,