[LTP] ftrace: Add common library for C implementation
linuxtestproject.agent@gmail.com
linuxtestproject.agent@gmail.com
Sat Jun 6 17:26:05 CEST 2026
Hi Praveen,
On Sat, 06 Jun 2026, Praveen K Pandey wrote:
> ftrace: Add common library for C implementation
--- [PATCH 1/5] ---
> +#include "tst_safe_macros.h"
> +#include "tst_safe_stdio.h"
ftrace_lib.h does not include tst_test.h. The SAFE_* macros expand to
code that calls tst_brk_(), which is declared only in tst_test.h.
Without it, ftrace_lib.c and every test file that includes ftrace_lib.h
will fail to compile with an implicit declaration error for tst_brk_().
Add #include "tst_test.h" to ftrace_lib.h.
> + debugfs_path = tst_tmpdir_mkpath("debugfs");
tst_tmpdir_mkpath() does not exist in LTP. A grep of the entire tree
finds no declaration or definition outside this file. This will produce
a linker error. The intended API is likely tst_get_tmpdir() combined
with a path construction step.
> +// Made with Bob
This line at the end of ftrace_lib.h looks like a leftover development
artifact. It must be removed before the patch is merged.
> + Fixes: #1282
The Fixes: tag must reference a valid git commit hash (e.g.
"Fixes: abc1234 (tracing: ...)"). A GitHub issue number is not valid
here. The same issue applies to all four commits that carry this tag
(patches 1-4).
--- [PATCH 2/5] ---
> + * [Description]
The [Description] header in the doc comment is deprecated. Remove it
and let the description text follow the opening /*\ line directly.
See rule 18 in the coding style checklist.
> + /* Small delay to let events trigger */
> + usleep(10000);
This is sleep-based synchronization (waiting for kernel trace events to
land in the buffer), which violates Ground Rule 2. The correct approach
is to poll the trace file until the expected content appears, using an
exponential-backoff loop.
--- [PATCH 3/5] ---
> + /* Small delay to ensure trace is written */
> + usleep(100000);
Same issue as in patch 2: sleep used to synchronize with the kernel
writing trace data. Replace with polling on the trace content.
--- [PATCH 4/5] ---
> +ftrace_regression01: ftrace_lib.o
> +ftrace_regression02: ftrace_lib.o
> +ftrace_stress_test: ftrace_lib.o
> +
> +LDLIBS += -lpthread
> +
> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
Three issues with the Makefile:
1. The shared helper object rules must appear AFTER the
generic_leaf_target.mk include, not before. MAKE_TARGETS is not
populated until that include is processed. Move the three
dependency lines after the include.
2. ftrace_lib.c is in the directory so the build framework will try to
build a standalone ftrace_lib binary. Add:
FILTER_OUT_MAKE_TARGETS := ftrace_lib
before the generic_leaf_target.mk include.
3. Only ftrace_stress_test.c uses pthreads. The global assignment
links -lpthread into ftrace_regression01 and ftrace_regression02
unnecessarily. Change to a per-target assignment:
ftrace_stress_test: LDLIBS += -lpthread
The new C binaries (ftrace_regression01, ftrace_regression02,
ftrace_stress_test) are not added to runtest/tracing. The existing
entries still point to the .sh versions, so the new C tests are built
but never executed by the LTP runner. Update runtest/tracing to include
entries for the new binaries.
--- [PATCH 5/5] ---
The commit is missing a Signed-off-by: tag, which is mandatory.
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
More information about the ltp
mailing list