Re: [LTP] [PATCH] pipe07: close /proc/self/fd after counting fds

Petr Vorel
2023-09-30T06:34:57Z

Hi Edward,

> Leaving the directory fd open will count against the max number of fds
> opened, so the final expected count will be off.

> Also, removed the halving / doubling of exp_num_pipes since it is
> redundant.

> Signed-off-by: Edward Liaw <edliaw@google.com>
> ---
> testcases/kernel/syscalls/pipe/pipe07.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)

> diff --git a/testcases/kernel/syscalls/pipe/pipe07.c b/testcases/kernel/syscalls/pipe/pipe07.c
> index 196485684..01f6b78f8 100644
> --- a/testcases/kernel/syscalls/pipe/pipe07.c
> +++ b/testcases/kernel/syscalls/pipe/pipe07.c
> @@ -45,6 +45,8 @@ static int record_open_fds(void)
> opened_fds[num_opened_fds++] = fd;
> }

> + SAFE_CLOSEDIR(dir);
IMHO this changes counting from 1020:

pipe07.c:56: TINFO: getdtablesize() = 1024
pipe07.c:60: TINFO: expected max fds to be opened by pipe(): 1020
pipe07.c:75: TPASS: errno == EMFILE (24)
pipe07.c:76: TPASS: exp_num_pipes * 2 == num_pipe_fds (1020)

to 1021, which leads to failure:

pipe07.c:58: TINFO: getdtablesize() = 1024
pipe07.c:62: TINFO: expected max fds to be opened by pipe(): 1021
pipe07.c:77: TPASS: errno == EMFILE (24)
pipe07.c:78: TFAIL: exp_num_pipes (1021) != num_pipe_fds (1020)

> +
> return num_opened_fds;
This is not elegant, but get's the correct count. Will it fail on AOSP?

return num_opened_fds + 1;
> }

> @@ -56,8 +58,8 @@ static void setup(void)
> tst_res(TINFO, "getdtablesize() = %d", max_fds);
> pipe_fds = SAFE_MALLOC(max_fds * sizeof(int));

> - exp_num_pipes = (max_fds - record_open_fds()) / 2;
> - tst_res(TINFO, "expected max fds to be opened by pipe(): %d", exp_num_pipes * 2);
> + exp_num_pipes = max_fds - record_open_fds();
> + tst_res(TINFO, "expected max fds to be opened by pipe(): %d", exp_num_pipes);

It'd be slightly more readable if this was in separate patch
(as it modifies the same variable), but it's up to you.

Kind regards,
Petr

> }

> static void run(void)
> @@ -73,7 +75,7 @@ static void run(void)
> } while (!TST_RET);

> TST_EXP_EQ_LI(errno, EMFILE);
> - TST_EXP_EQ_LI(exp_num_pipes * 2, num_pipe_fds);
> + TST_EXP_EQ_LI(exp_num_pipes, num_pipe_fds);

> for (int i = 0; i < num_pipe_fds; i++)
> SAFE_CLOSE(pipe_fds[i]);