Re: [LTP] [PATCH 4/5] Add pidfd_getfd01 test

[Petr Vorel <pvorel@suse.cz>]

Hi Xu,

...
> +++ b/testcases/kernel/syscalls/pidfd_getfd/pidfd_getfd01.c
...
> +	remotefd = TST_RET;
> +	flag = fcntl(remotefd, F_GETFD);
> +	if (flag == -1)
> +		tst_brk(TFAIL | TERRNO, "fcntl(F_GETFD) failed");
Just:
flag = SAFE_FCNTL(remotefd, F_GETFD);

> +	if (!(flag & FD_CLOEXEC))
> +		tst_res(TFAIL, "pidfd_getfd() didn't set close-on-exec flag");
> +
> +	TEST(kcmp(getpid(), pid, KCMP_FILE, remotefd, targetfd));
> +	if (TST_RET != 0)
> +		tst_res(TFAIL, "pidfd_getfd() didn't get the same open file description");
Maybe just:
       TST_EXP_PASS_SILENT(kcmp(getpid(), pid, KCMP_FILE, remotefd, targetfd));

       if (!TST_PASS)
               return;
Although your version is more descriptive.

> +
> +	TST_CHECKPOINT_WAKE(0);
> +	SAFE_CLOSE(remotefd);
> +
> +	tst_res(TPASS, "pidfd_getfd(%d, %d, 0) passed", pidfd, targetfd);
> +	SAFE_CLOSE(pidfd);
Shouldn't be pidfd closed in cleanup? In case fcntl() fails it's kept open.
> +	SAFE_CLOSE(fds[0]);
The same is for fds, which is already static.

These are very minor and you can change it before merge.

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

Kind regards,
Petr

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