Re: [LTP] pty09: Cap RLIMIT_NOFILE-based slave PTY opens

Re: [LTP] pty09: Cap RLIMIT_NOFILE-based slave PTY opens

[linuxtestproject.agent]

Hi Runli,

On Thu, 09 Jul 2026 15:49:23 +0800, Runli wrote:
> pty09: Cap RLIMIT_NOFILE-based slave PTY opens

>  	max_pid_num = limit.rlim_cur - count;
> +	if (max_pid_num > MAX_SLAVE_OPENS)
> +		max_pid_num = MAX_SLAVE_OPENS;
>  
>  	tst_res(TINFO, "Available number of pids: %u", max_pid_num);

After the cap fires, `max_pid_num` holds 4096 -- the test's self-imposed
limit -- but the TINFO label still says "Available number", which implies
the system's available FD count.

On a system where RLIMIT_NOFILE - open_fds is, say, 1048000, the output
would read "Available number of pids: 4096", which could mislead anyone
comparing it against the system limit.

Consider printing the uncapped available count and a separate note when
the cap applies, e.g.:

  tst_res(TINFO, "Available number of pids: %u (capped at %d)",
          max_pid_num, MAX_SLAVE_OPENS);

or split the TINFO into two calls -- one for the real available count
(computed before the cap), one confirming the cap was applied.

Verdict - Needs revision

Pre-existing issues:

1. `count_avail_pid()` and the variable `max_pid_num` both say "pid"
   but the code tracks file-descriptor slots, not PIDs.

2. In run(): `tst_res(TPASS, "pty has been opened %d times", max_pid_num)`
   uses %d for an unsigned int argument.

3. In run(): `int slavefd[max_pid_num]` is a VLA. If max_pid_num == 0
   (all FD slots occupied) the zero-length VLA is undefined behaviour.

---
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

Re: [LTP] pty09: Cap RLIMIT_NOFILE-based slave PTY opens

[Cyril Hrubis]

Hi!
>   tst_res(TINFO, "Available number of pids: %u (capped at %d)",
>           max_pid_num, MAX_SLAVE_OPENS);

Agree here, the message was confusing and this one looks better.

-- 
Cyril Hrubis
chrubis@suse.cz

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

[LTP] [PATCH v2] pty09: Cap RLIMIT_NOFILE-based slave PTY opens

[Runli via ltp]

pty09 derives the number of slave PTY opens from RLIMIT_NOFILE. When the
limit is very large, for example 1048576, the test tries to open nearly
one million slave PTYs on the same master. That makes the workload depend
on the caller's file descriptor limit instead of the PTY behavior being
tested, and can turn a working PTY open path into a timeout-driven TBROK.

The test only needs to verify that a slave pseudo-terminal can be opened
multiple times in parallel. Cap the number of slave opens at 4096 so the
test remains representative and deterministic even when RLIMIT_NOFILE is
set unusually high.

Fixes: 20aa13dbd8a68 ("Add pty09 test")
Signed-off-by: Runli <mingyu.he@shopee.com>
---
Changes in v2:
- Print the uncapped available count before applying the cap.
- Add a separate TINFO message when the open count is capped.

 testcases/kernel/pty/pty09.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/testcases/kernel/pty/pty09.c b/testcases/kernel/pty/pty09.c
index f3ade2381..80da475e9 100644
--- a/testcases/kernel/pty/pty09.c
+++ b/testcases/kernel/pty/pty09.c
@@ -14,6 +14,8 @@
 
 static int masterfd = -1;
 
+#define MAX_SLAVE_OPENS 4096
+
 static unsigned int count_avail_pid(void)
 {
 	DIR *dir;
@@ -34,6 +36,12 @@ static unsigned int count_avail_pid(void)
 
 	tst_res(TINFO, "Available number of pids: %u", max_pid_num);
 
+	if (max_pid_num > MAX_SLAVE_OPENS) {
+		max_pid_num = MAX_SLAVE_OPENS;
+		tst_res(TINFO, "Number of pids is too large, capped at: %u",
+			max_pid_num);
+	}
+
 	return max_pid_num;
 }
 
-- 
2.47.0

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

Re: [LTP] pty09: Cap RLIMIT_NOFILE-based slave PTY opens

[linuxtestproject.agent]

Hi Runli,

On Tue, 14 Jul 2026, Runli wrote:
> pty09: Cap RLIMIT_NOFILE-based slave PTY opens

> +	if (max_pid_num > MAX_SLAVE_OPENS) {
> +		max_pid_num = MAX_SLAVE_OPENS;
> +		tst_res(TINFO, "Number of pids is too large, capped at: %u",
> +			max_pid_num);
> +	}

The value being capped is derived from RLIMIT_NOFILE (open file descriptor
slots), not process IDs.  "Number of pids is too large" describes the wrong
resource.

The commit message itself uses the accurate term ("slave opens").  Suggest
aligning the TINFO message with that:

    tst_res(TINFO, "Number of slave opens is too large, capped at: %u",
            max_pid_num);

Pre-existing issue: the containing function count_avail_pid() and its local
variable max_pid_num carry the same "pid" misnomer for what is really an
open-fd-slot count.  Not introduced by this patch, but worth noting.

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