Re: [LTP] [PATCH v2] Rewrite ftrace_regression tests with new C API

[Andrea Cervesato via ltp <ltp@lists.linux.it>]

Hi lufei,

> +/**
> + * file_contains_line - Check if a file ocntains specific line, strictly

Typo: "ocntains" -> "contains".

> +static inline int file_contains(const char *path, const char *str)
> +{
> +	char *buf = SAFE_MALLOC(FTRACE_FILE_BUF_SIZE);
> +
> +	buf[0] = '\0';
> +	SAFE_FILE_SCANF(path, "%1048575[^\x01]", buf);

The hardcoded width 1048575 must be kept in sync with FTRACE_FILE_BUF_SIZE - 1
manually. Also, allocating 1 MB on each call is heavy — file_contains() is
called twice in ftrace_regression02. Consider a simpler file reading approach.

> diff --git a/testcases/kernel/tracing/ftrace_test/ftrace_regression01.c b/testcases/kernel/tracing/ftrace_test/ftrace_regression01.c
> new file mode 100644
[...]
> +	for (i = 0; i < LOOP; i++) {
> +		SAFE_FILE_PRINTF(STACK_TRACER_PATH, "1");
> +		SAFE_FILE_PRINTF(TRACE_OPTIONS, "userstacktrace");
[...]
> +		SAFE_FILE_PRINTF(page_fault_path, "1");
> +	}

The old shell script saved and restored the page fault enable value after
the loop. This version writes "1" but never restores it. Unmounting debugfs
does not reset kernel tracing state. Please add the page_fault_path to
.save_restore or save/restore it manually.

> diff --git a/testcases/kernel/tracing/ftrace_test/ftrace_regression02.c b/testcases/kernel/tracing/ftrace_test/ftrace_regression02.c
> new file mode 100644
[...]
> +/*\
> + *
> + * Check signal:signal_generate gives 2 more fields: grp res
> + *
> + */

Remove the blank lines before/after the description inside the /*\ block.

> +	if (file_contains(TRACE_FILE, "grp=") && file_contains(TRACE_FILE, "res="))

The old test used 'grp=[0-9] res=[0-9]' which verified both fields appear on
the same line with digit values. This only checks for substrings independently,
which is less strict.

Also, there is no .gitignore in the ftrace_test directory. Please add one
with entries for ftrace_regression01 and ftrace_regression02.

Regards,
--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp