Re: [LTP] mremap01: Convert to new API

[linuxtestproject.agent@gmail.com]

Hi Andrea,

On Fri, 7 Aug 2026, Andrea Cervesato wrote:
> mremap01: Convert to new API

--- [PATCH 1/7] ---

> static int verify_file(void)
> {
> [...]
> 			tst_res(TFAIL, "file[%zu] == 0x%02x, expected 0x%02x",
> 				off, (unsigned char)got, (unsigned char)off);
> 			return 1;
> [...]
> 	if (verify_file())
> 		tst_res(TFAIL, "mremap()'d region did not sync to the file");

Could verify_file() report the final result directly instead of returning
pass/fail status to run()? A mismatch currently produces both the precise
byte failure and a second generic TFAIL for the same check.

--- [PATCH 3/7] ---

>  * - Obtain an unmapped address via ``tst_get_bad_addr()``.
> [...]
> 	bad_addr = tst_get_bad_addr(NULL);

Could this use an address that is actually unmapped, or describe the
partial-VMA case that is really tested? tst_get_bad_addr() creates and
retains a one-page PROT_NONE mapping, so old_address itself is mapped.

On Linux 7.2 this returns EFAULT because memsize extends beyond that
one-page VMA, not because old_address points to an unmapped region. The
description, algorithm, and commit message therefore claim different
coverage from the executed path.

--- [PATCH 4/7] ---

> Rewrite the test to use TST_EXP_FAIL_PTR_VOID() to verify that
> mremap() fails with MAP_FAILED and ENOMEM when growing an existing
> SysV shared memory mapping in place
> [...]
> Switch the Makefile from libltpipc to libltpnewipc

Could the commit message be updated to match the patch? The code replaces
SysV shared memory with a private anonymous mapping and removes the IPC
library dependency; it does not use libltpnewipc or GETIPCKEY().

--- [PATCH 5/7] ---

> Convert the mremap05 test case from the legacy LTP API to the new
> tst_test API.

Could the body explain why this semantic rewrite is needed and mention
that the positive MREMAP_FIXED cases are split into the later dedicated
test? It currently only restates the subject and does not give the
motivation or explain the series dependency.

--- [PATCH 6/7] ---

> static int check_pattern(char *addr, size_t pages)
> {
> [...]
> 		if (got != exp)
> 			return 1;
> [...]
> 	else if (check_pattern(ret, tc->new_pages))
> 		tst_res(TFAIL, "%s: pattern mismatch", tc->msg);

Could check_pattern() report the result where the mismatch is detected
instead of propagating pass/fail through its return value? Reporting the
offset and observed value there would also make the failure actionable
rather than producing only "pattern mismatch".

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