Re: [LTP] ftrace: Add common library for C implementation

[linuxtestproject.agent@gmail.com]

Hi Praveen,

On Sun, 07 Jun 2026 10:11:56 +0530, Praveen K Pandey wrote:
> ftrace: Add common library for C implementation

--- [PATCH 1/5] ---

> +tracing_path = malloc(PATH_MAX);
> +if (!tracing_path)
> +tst_brk(TBROK | TERRNO, "malloc failed");
> +
> +snprintf(tracing_path, PATH_MAX, "%s/tracing", debugfs_path);

The blank line between `tst_brk()` and `snprintf()` has a trailing
tab. The same pattern appears on many blank lines throughout
ftrace_lib.c (at least lines 56, 78, 93, 97, 111, 115, 118, 121,
135, 138, 141, 150, 154, 157, 160, 163, 171, 175, 178 and more).
`make check` would catch all of these.

--- [PATCH 2/5] ---

The commit body says:

  This test validates that function tracing works correctly by:
  - Enabling function tracing
  - Verifying trace output contains function calls
  - Checking for kernel crashes or hangs

The test does none of that. It enables the userstacktrace option
together with page fault events and repeats the loop to check the
kernel does not panic (regression for commit 1dbd195). The body
needs to describe what the code actually does.

--- [PATCH 3/5] ---

Same problem with the commit body:

  This test validates that function graph tracing works correctly by:
  - Enabling function_graph tracer
  - Verifying trace output contains function graph format
  - Checking for kernel crashes or hangs

The test verifies that the signal:signal_generate tracepoint exposes
'grp=' and 'res=' fields. This description appears to belong to a
different test.

> +/* This test requires kernel >= 3.2 */
> +if (tst_kvercmp(3, 2, 0) < 0)
> +tst_brk(TCONF, "This test requires kernel >= 3.2.0");

Kernel version gating must use `.min_kver = "3.2"` in
`struct tst_test` rather than an inline `tst_kvercmp()` call.
See c-tests rule 9.

--- [PATCH 4/5] ---

> +/* Check for test duration parameter */
> +if (tst_parse_int(getenv("LTP_TIMEOUT_MUL"), &test_duration,
> +  1, INT_MAX))
> +test_duration = DEFAULT_TEST_DURATION;

`LTP_TIMEOUT_MUL` is a floating-point CI timeout multiplier (e.g.
"1.5"), not an integer duration. `tst_parse_int()` will silently
fail for float values, leaving the duration at 60 s. If it happens
to be set to a small integer like "2", the stress run would last
only 2 s instead of 60 s. Either drop this override or use a
dedicated environment variable for duration control.

Verdict: Needs revision

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

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