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/6] ---

> 	if (verify_file())
> 		tst_res(TFAIL, "mremap()'d region did not sync to the file");
> 	else
> 		tst_res(TPASS, "Functionality of mremap() is correct");

Could verify_file() report the final result directly instead of returning
test status to run()? It already emits TFAIL for a mismatched byte, so the
current failure path reports a second TFAIL at the call site.

--- [PATCH 3/6] ---

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

Could this create and unmap a memsize-sized range instead?
tst_get_bad_addr() maps one PROT_NONE page and returns that still-mapped
page. Thus old_address itself is mapped, and EFAULT depends on old_size
extending beyond it, contrary to the description and commit message.

--- [PATCH 4/6] ---

> -LTPLIBS = ipc
> +LTPLIBS = newipc
> -mremap04: LTPLDLIBS  = -lltpipc
> +mremap04: LTPLDLIBS  = -lltpnewipc

Could both IPC library assignments be removed? The rewritten test removes
all SysV IPC usage, does not include tst_newipc.h or call GETIPCKEY(), and no
other test in this directory uses either IPC library.

The commit message also says that the new test uses SysV shared memory and
GETIPCKEY(). Could it describe the actual anonymous mmap rewrite instead?

--- [PATCH 5/6] ---

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

Could the body explain why this rewrite removes the two successful
MREMAP_FIXED cases from mremap05 and splits them into a separate test? The
current text only restates the subject and leaves the coverage change
unexplained.

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

> 	if (ret != new_address)
> 		tst_res(TFAIL, "%s: ret %p, expected %p", tc->msg, ret, new_address);
> 	else if (check_pattern(tc->msg, ret, tc->new_pages) == 0)
> 		tst_res(TPASS, "%s", tc->msg);

Could check_pattern() report the final TPASS or TFAIL itself, or could the
content check remain in run()? It currently emits TFAIL and propagates a
status back to run() to decide whether to emit TPASS.

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