Re: [LTP] [PATCH] [PATCH] Add hugemmap37, migrated task-size-overrun.c from libhugetlbfs v3

[Andrea Cervesato via ltp <ltp@lists.linux.it>]

Hi Pavithra,

The commit message body describes "Changes in v3" but doesn't explain
why this test is being migrated. The v3 changelog belongs below the ---
line; the body should describe what the test does and why it's useful.

> +/*
> + * Copyright (C) 2005-2006 David Gibson & Adam Litke, IBM Corporation.
> + */

This is a new file, so it needs a copyright line with the current year
and your name, e.g.:
Copyright (c) 2026 Pavithra <pavrampu@linux.ibm.com>

> +/*\
> + *[Descripiton]

Typo ("Descripiton"), and [Description] is deprecated — remove the tag
entirely. The doc text alone is sufficient.

> +#define MAPS_BUF_SZ 4096
> +#define _LARGEFILE64_SOURCE

_LARGEFILE64_SOURCE must be defined before any includes to take effect.
It also appears unused here — drop it.

> +static int fd;

Must be initialized to -1. Zero is a valid fd (stdin).

> +	tst_res(TINFO, "Last map: at 0x%lx-0x%lx\n", start, end);

[...]

> +	tst_res(TINFO, "Binary searching for task size PFNs 0x%lx..0x%lx\n", low, high);

[...]

> +		tst_res(TINFO, "Map succeeded at 0x%lx\n", addr);

[...]

> +	tst_res(TINFO, "TASK_SIZE = 0x%lx\n", task_size);

Drop the trailing \n in all tst_res() calls — the framework adds
newlines automatically.

> +	p = mmap((void *)straddle_addr, 2*hpage_size, PROT_READ|PROT_WRITE,
> +		MAP_SHARED, fd, 0);
> +	if (p == (void *)straddle_addr)
> +		tst_res(TFAIL, "Apparently suceeded in mapping across TASK_SIZE boundary");

If mmap succeeds at a different address, the mapping is never unmapped
(resource leak). Also need to munmap when it does land at straddle_addr
after reporting TFAIL. Typo: "suceeded" -> "succeeded".

> +	p = mmap((void *)straddle_addr, 2*hpage_size, PROT_READ|PROT_WRITE,
> +		MAP_SHARED|MAP_FIXED, fd, 0);
> +	if (p != MAP_FAILED)
> +		tst_res(TFAIL, "Apparently suceeded in mapping across TASK_SIZE boundary");

Same here — munmap the mapping after TFAIL. Same typo.

> +	tst_res(TPASS, "Test passed!");

This fires unconditionally, even after TFAIL was reported above. Each
sub-case (non-MAP_FIXED and MAP_FIXED) should have its own TPASS/TFAIL
pair, so the result is per-case rather than a blanket pass.

> +static void cleanup(void)
> +{
> +	if (fd > 0)
> +		SAFE_CLOSE(fd);
> +}

Use "fd != -1", not "fd > 0". The > 0 check silently skips fd 0.

Regards,
--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp