Re: [LTP] [PATCH] syscalls/pipe07: Rewrite the test using new LTP API

[Petr Vorel <pvorel@suse.cz>]

Hi Avinesh,

generally LGTM, thank you.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

BTW it's funny that when run with valgrind, it fails because valgrind opens some
file descriptors:

$ valgrind ./pipe07
...
pipe07.c:45: TINFO: getdtablesize() = 1024
pipe07.c:49: TINFO: open fds before pipe() calls: 10
pipe07.c:54: TINFO: expected max fds to be opened by pipe(): 1014
==1629480== Warning: invalid file descriptor 1030 in syscall pipe2()
pipe07.c:69: TPASS: errno == EMFILE (24)
pipe07.c:70: TFAIL: exp_num_pipe_fds (1014) != num_pipe_fds (1020)

> +static void record_open_fds(void)
nit: num_opened_fds is used only in setup(), I'd personally return int
and store variable in setup().

>  {
> +	DIR *dir;
> +	struct dirent *ent;
> +	int fd;

> -	min = getdtablesize() - rec_fds_max;
> +	dir = SAFE_OPENDIR("/proc/self/fd");
...

> +static void run(void)
>  {
...
> +	do {
> +		TEST(pipe(fds));
> +		if (TST_RET != -1) {
nit: wouldn't be safer to use: if (!TST_RET) (i.e. for TST_RET == 0)
(we check that return on error is exactly -1, not > 0)

Kind regards,
Petr

> +			pipe_fds[num_pipe_fds++] = fds[0];
> +			pipe_fds[num_pipe_fds++] = fds[1];
>  		}
> +	} while (TST_RET != -1);

...

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