Re: tst_kvercmp: Factor out error handling

Hi Petr,

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

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

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

Could this explain the distinction between minimum- and maximum-version
checks without referring to the next patch? Commit messages in a series
should be self-contained.

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

> +	.max_kver = "7.1",

Could only the FAN_REPORT_PIDFD | FAN_REPORT_TID case be adjusted or split
instead? This skips every case on Linux 7.2 and newer, including
FAN_REPORT_PIDFD | FAN_REPORT_FID | FAN_REPORT_DFID_NAME, whose expected
behavior did not change. That permanently removes its regression coverage on
newer kernels.

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

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

How does this exercise the successful min/max path on current kernels? It
exits with TCONF on every kernel newer than 5.0, and runtest.sh accepts TCONF
as success without calling do_test(). Could the upper bound be made safely
higher than the running kernel, or otherwise deterministic?

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

> +/**
> + * tst_kver_cmp() - Compare two kernel versions, versions passed by 3 integers.
> + *
> + * @a1: First kernel major version.
> + * @a2: First kernel minor version.
> + * @a3: First kernel patch level.
> + * @b1: Second kernel major version.
> + * @b2: Second kernel minor  version.
> + * @b3: Second kernel patch level.
> + */

Could this document the return value? Callers need to know that a negative
result means the first version is older, zero means equal, and a positive
result means newer.

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

Could the reusable comparison need be explained without referring to the next
patch? Commit messages in a series should be self-contained.

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

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

Could this describe the host-tool link or build-order problem solved by the
rule? The current body relies on the next patch and does not explain why host
targets need MAKE_DEPS.

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

> +	if (min_kver && max_kver) {
> +		if (tst_kver_cmp(a1, a2, a3, b1, b2, b3) < 0) {

Could this apply the same two-component max_kver semantics as
check_max_kver()? For example, min_kver "7.1.5" and max_kver "7.1" is rejected
as 7.1.5 > 7.1.0, although the API defines max_kver "7.1" to include every
7.1.x kernel.

> +include $(top_srcdir)/include/mk/testcases.mk
>
> +metaparse: HOST_CFLAGS		+= -I$(abs_srcdir)/../include -L$(abs_builddir)/../lib
> +metaparse: HOST_LDLIBS		+= -lltp

How will this work for cross builds? metaparse is built with HOSTCC, while
testcases.mk builds libltp.a with the target CC. A host linker cannot consume
a target-architecture archive. Could the parsing and comparison code used by
metaparse be built with HOSTCC or shared without linking the target 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