Re: [LTP] tst_kvercmp: Factor out error handling

[linuxtestproject.agent@gmail.com]

Hi Petr,

On Aug 5, 2026, Petr Vorel wrote:
> tst_kvercmp: Factor out error handling

--- [PATCH 1/9] ---

> static int _tst_parse_kver(const char *str_kver, int *v1, int *v2, int *v3)

Could this be renamed to parse_kver? File-scope identifiers beginning with an
underscore are reserved by C, and LTP's coding rules prohibit such names.

--- [PATCH 2/9] ---

> This is a preparation for struct tst_test max_kver member in the next
> commit.

Could the motivation be stated without referring to the next patch? Each
commit message should stand on its own rather than depend on series position.

--- [PATCH 4/9] ---

> 	if (TST_RET != -1) {
> 		tst_res(TFAIL, "creat() succeeded unexpectedly");
> 		return;
> 	}

Could this path kill and reap pid before returning? Removing the version gate
makes it reachable on affected 6.11 and 6.12 kernels, but the child remains
paused indefinitely. The framework then blocks in tst_reap_children() until
the watchdog terminates the test instead of returning the intended TFAIL.

> 	TEST(execve(TEST_APP, argv, environ));

How is the writer child released when execve() succeeds on an affected
kernel? The test process is replaced by execve_child, which reports TFAIL and
exits, while the writer remains blocked in TST_CHECKPOINT_WAKE_AND_WAIT(0) and
becomes an orphan.

--- [PATCH 5/9] ---

> 	.max_kver = "7.1",

Could the test keep running and only adjust the expected result of
FAN_REPORT_PIDFD | FAN_REPORT_TID on kernels supporting thread pidfds? This
test-wide limit also removes the
FAN_REPORT_PIDFD | FAN_REPORT_FID | FAN_REPORT_DFID_NAME case on 7.2 and all
newer kernels, and that exact combination is not tested elsewhere in LTP.

--- [PATCH 6/9] ---

> 	.min_kver = "4.4",
> 	.max_kver = "5.0",

How can this exercise do_test() on current CI kernels? The current kernel must
lie between the bounds; min_kver being less than max_kver is not sufficient.
With this maximum, including on stable 7.2, the self-test only reports TCONF
and never tests the new success path.

--- [PATCH 7/9] ---

> int tst_kver_cmp(int a1, int a2, int a3, int b1, int b2, int b3);

Could the new public function's kernel-doc include a Return: section
describing the comparison result?

> This will be heavily used in metaparse.c (speedup of metadata
> generation) in the next commit.

Could this motivation be stated without referring to the next patch? Each
commit message should stand on its own.

--- [PATCH 8/9] ---

> Will be used for metadata.c in the next commit.

Could the commit message instead explain why host targets need access to
MAKE_DEPS without depending on a later patch?

--- [PATCH 9/9] ---

> metaparse: HOST_LDLIBS		+= -lltp

How can this work for a cross-compilation? metaparse is built with HOSTCC, but
-lltp resolves to lib/libltp.a built with the target CC, so the host linker
cannot consume the archive. Could the version helper be compiled with HOSTCC,
or moved into an architecture-independent host library?

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