| Message ID | 1644399738-2155-1-git-send-email-xuyang2018.jy@fujitsu.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/2] syscalls/pidfd_open: Simplify code | expand |
Hi All It seems I should use TST_EXP_FD_SILENT instead of TST_EXP_PID_SILENT. Best Regards Yang Xu > 1) make use of TST_EXP_FAIL2 or TST_EXP_PID_SILENT macros > 2) remove min_kver check and use pidfd_open_supported() > 3) Add docparse formatting > > Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com> > --- > include/lapi/pidfd_open.h | 8 +++-- > .../kernel/syscalls/pidfd_open/pidfd_open01.c | 22 +++++++----- > .../kernel/syscalls/pidfd_open/pidfd_open02.c | 34 +++++++------------ > .../kernel/syscalls/pidfd_open/pidfd_open03.c | 16 ++++++--- > 4 files changed, 44 insertions(+), 36 deletions(-) > > diff --git a/include/lapi/pidfd_open.h b/include/lapi/pidfd_open.h > index 9806c73d4..5cf10933e 100644 > --- a/include/lapi/pidfd_open.h > +++ b/include/lapi/pidfd_open.h > @@ -9,11 +9,15 @@ > > #include<sys/syscall.h> > #include<sys/types.h> > - > #include "lapi/syscalls.h" > - > #include "config.h" > > +static inline void pidfd_open_supported(void) > +{ > + /* allow the tests to fail early */ > + tst_syscall(__NR_pidfd_open); > +} > + > #ifndef HAVE_PIDFD_OPEN > static inline int pidfd_open(pid_t pid, unsigned int flags) > { > diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c > index f40e9b624..ed9b82637 100644 > --- a/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c > +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c > @@ -1,11 +1,15 @@ > // SPDX-License-Identifier: GPL-2.0-or-later > /* > * Copyright (c) 2020 Viresh Kumar<viresh.kumar@linaro.org> > + */ > + > +/*\ > + * [Description] > * > - * Description: > * Basic pidfd_open() test: > - * 1) Fetch the PID of the current process and try to get its file descriptor. > - * 2) Check that the close-on-exec flag is set on the file descriptor. > + * > + * - Fetch the PID of the current process and try to get its file descriptor. > + * - Check that the close-on-exec flag is set on the file descriptor. > */ > > #include<unistd.h> > @@ -17,10 +21,7 @@ static void run(void) > { > int flag; > > - TEST(pidfd_open(getpid(), 0)); > - > - if (TST_RET == -1) > - tst_brk(TFAIL | TTERRNO, "pidfd_open(getpid(), 0) failed"); > + TST_EXP_PID_SILENT(pidfd_open(getpid(), 0), "pidfd_open(getpid(), 0)"); > > flag = fcntl(TST_RET, F_GETFD); > > @@ -35,7 +36,12 @@ static void run(void) > tst_res(TPASS, "pidfd_open(getpid(), 0) passed"); > } > > +static void setup(void) > +{ > + pidfd_open_supported(); > +} > + > static struct tst_test test = { > - .min_kver = "5.3", > + .setup = setup, > .test_all = run, > }; > diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c > index dc86cae7a..93a61a51d 100644 > --- a/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c > +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c > @@ -1,14 +1,21 @@ > // SPDX-License-Identifier: GPL-2.0-or-later > /* > * Copyright (c) 2020 Viresh Kumar<viresh.kumar@linaro.org> > + */ > + > +/*\ > + * [Description] > + * > + * Tests basic error handling of the pidfd_open syscall. > * > - * Description: > - * Basic pidfd_open() test to test invalid arguments. > + * - ESRCH the process specified by pid does not exist > + * - EINVAL pid is not valid > + * - EINVAL flags is not valid > */ > #include "tst_test.h" > #include "lapi/pidfd_open.h" > > -pid_t expired_pid, my_pid, invalid_pid = -1; > +static pid_t expired_pid, my_pid, invalid_pid = -1; > > static struct tcase { > char *name; > @@ -23,6 +30,7 @@ static struct tcase { > > static void setup(void) > { > + pidfd_open_supported(); > expired_pid = tst_get_unused_pid(); > my_pid = getpid(); > } > @@ -31,27 +39,11 @@ static void run(unsigned int n) > { > struct tcase *tc =&tcases[n]; > > - TEST(pidfd_open(*tc->pid, tc->flags)); > - > - if (TST_RET != -1) { > - SAFE_CLOSE(TST_RET); > - tst_res(TFAIL, "%s: pidfd_open succeeded unexpectedly (index: %d)", > - tc->name, n); > - return; > - } > - > - if (tc->exp_errno != TST_ERR) { > - tst_res(TFAIL | TTERRNO, "%s: pidfd_open() should fail with %s", > - tc->name, tst_strerrno(tc->exp_errno)); > - return; > - } > - > - tst_res(TPASS | TTERRNO, "%s: pidfd_open() failed as expected", > - tc->name); > + TST_EXP_FAIL2(pidfd_open(*tc->pid, tc->flags), tc->exp_errno, > + "pidfd_open with %s", tc->name); > } > > static struct tst_test test = { > - .min_kver = "5.3", > .tcnt = ARRAY_SIZE(tcases), > .test = run, > .setup = setup, > diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c > index 48470e5e1..f41afa2fc 100644 > --- a/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c > +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c > @@ -1,8 +1,11 @@ > // SPDX-License-Identifier: GPL-2.0-or-later > /* > * Copyright (c) 2020 Viresh Kumar<viresh.kumar@linaro.org> > + */ > + > +/*\ > + * [Description] > * > - * Description: > * This program opens the PID file descriptor of the child process created with > * fork(). It then uses poll to monitor the file descriptor for process exit, as > * indicated by an EPOLLIN event. > @@ -27,11 +30,9 @@ static void run(void) > exit(EXIT_SUCCESS); > } > > - TEST(pidfd_open(pid, 0)); > + TST_EXP_PID_SILENT(pidfd_open(pid, 0), "pidfd_open(%d, 0)", pid); > > fd = TST_RET; > - if (fd == -1) > - tst_brk(TFAIL | TTERRNO, "pidfd_open() failed"); > > TST_CHECKPOINT_WAKE(0); > > @@ -49,8 +50,13 @@ static void run(void) > tst_res(TPASS, "pidfd_open() passed"); > } > > +static void setup(void) > +{ > + pidfd_open_supported(); > +} > + > static struct tst_test test = { > - .min_kver = "5.3", > + .setup = setup, > .test_all = run, > .forks_child = 1, > .needs_checkpoints = 1,
Hi! > diff --git a/include/lapi/pidfd_open.h b/include/lapi/pidfd_open.h > index 9806c73d4..5cf10933e 100644 > --- a/include/lapi/pidfd_open.h > +++ b/include/lapi/pidfd_open.h > @@ -9,11 +9,15 @@ > > #include <sys/syscall.h> > #include <sys/types.h> > - > #include "lapi/syscalls.h" > - > #include "config.h" > > +static inline void pidfd_open_supported(void) > +{ > + /* allow the tests to fail early */ > + tst_syscall(__NR_pidfd_open); > +} > + > #ifndef HAVE_PIDFD_OPEN > static inline int pidfd_open(pid_t pid, unsigned int flags) > { > diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c > index f40e9b624..ed9b82637 100644 > --- a/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c > +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c > @@ -1,11 +1,15 @@ > // SPDX-License-Identifier: GPL-2.0-or-later > /* > * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org> > + */ > + > +/*\ > + * [Description] > * > - * Description: > * Basic pidfd_open() test: > - * 1) Fetch the PID of the current process and try to get its file descriptor. > - * 2) Check that the close-on-exec flag is set on the file descriptor. > + * > + * - Fetch the PID of the current process and try to get its file descriptor. > + * - Check that the close-on-exec flag is set on the file descriptor. > */ > > #include <unistd.h> > @@ -17,10 +21,7 @@ static void run(void) > { > int flag; > > - TEST(pidfd_open(getpid(), 0)); > - > - if (TST_RET == -1) > - tst_brk(TFAIL | TTERRNO, "pidfd_open(getpid(), 0) failed"); > + TST_EXP_PID_SILENT(pidfd_open(getpid(), 0), "pidfd_open(getpid(), 0)"); Techincally the return value is a fd so this should be TST_EXP_FD_SILENT(). > flag = fcntl(TST_RET, F_GETFD); > > @@ -35,7 +36,12 @@ static void run(void) > tst_res(TPASS, "pidfd_open(getpid(), 0) passed"); > } > > +static void setup(void) > +{ > + pidfd_open_supported(); > +} > + > static struct tst_test test = { > - .min_kver = "5.3", > + .setup = setup, > .test_all = run, > }; > diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c > index dc86cae7a..93a61a51d 100644 > --- a/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c > +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c > @@ -1,14 +1,21 @@ > // SPDX-License-Identifier: GPL-2.0-or-later > /* > * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org> > + */ > + > +/*\ > + * [Description] > + * > + * Tests basic error handling of the pidfd_open syscall. > * > - * Description: > - * Basic pidfd_open() test to test invalid arguments. > + * - ESRCH the process specified by pid does not exist > + * - EINVAL pid is not valid > + * - EINVAL flags is not valid > */ > #include "tst_test.h" > #include "lapi/pidfd_open.h" > > -pid_t expired_pid, my_pid, invalid_pid = -1; > +static pid_t expired_pid, my_pid, invalid_pid = -1; > > static struct tcase { > char *name; > @@ -23,6 +30,7 @@ static struct tcase { > > static void setup(void) > { > + pidfd_open_supported(); > expired_pid = tst_get_unused_pid(); > my_pid = getpid(); > } > @@ -31,27 +39,11 @@ static void run(unsigned int n) > { > struct tcase *tc = &tcases[n]; > > - TEST(pidfd_open(*tc->pid, tc->flags)); > - > - if (TST_RET != -1) { > - SAFE_CLOSE(TST_RET); > - tst_res(TFAIL, "%s: pidfd_open succeeded unexpectedly (index: %d)", > - tc->name, n); > - return; > - } > - > - if (tc->exp_errno != TST_ERR) { > - tst_res(TFAIL | TTERRNO, "%s: pidfd_open() should fail with %s", > - tc->name, tst_strerrno(tc->exp_errno)); > - return; > - } > - > - tst_res(TPASS | TTERRNO, "%s: pidfd_open() failed as expected", > - tc->name); > + TST_EXP_FAIL2(pidfd_open(*tc->pid, tc->flags), tc->exp_errno, > + "pidfd_open with %s", tc->name); > } > > static struct tst_test test = { > - .min_kver = "5.3", > .tcnt = ARRAY_SIZE(tcases), > .test = run, > .setup = setup, > diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c > index 48470e5e1..f41afa2fc 100644 > --- a/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c > +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c > @@ -1,8 +1,11 @@ > // SPDX-License-Identifier: GPL-2.0-or-later > /* > * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org> > + */ > + > +/*\ > + * [Description] > * > - * Description: > * This program opens the PID file descriptor of the child process created with > * fork(). It then uses poll to monitor the file descriptor for process exit, as > * indicated by an EPOLLIN event. > @@ -27,11 +30,9 @@ static void run(void) > exit(EXIT_SUCCESS); > } > > - TEST(pidfd_open(pid, 0)); > + TST_EXP_PID_SILENT(pidfd_open(pid, 0), "pidfd_open(%d, 0)", pid); Here as well. Otherwise it looks good: Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Hi Xu, Reviewed-by: Petr Vorel <pvorel@suse.cz> Nice cleanup. +1 for Cyril's note to TST_EXP_FD_SILENT(). > diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c > +static void setup(void) > +{ > + pidfd_open_supported(); > +} > + > static struct tst_test test = { > - .min_kver = "5.3", > + .setup = setup, > .test_all = run, why not just .setup = pidfd_open_supported; ? ... > diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c > index 48470e5e1..f41afa2fc 100644 > --- a/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c > +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c ... > +static void setup(void) > +{ > + pidfd_open_supported(); > +} > + And here as well. Kind regards, Petr > static struct tst_test test = { > - .min_kver = "5.3", > + .setup = setup, > .test_all = run, > .forks_child = 1, > .needs_checkpoints = 1,
Hi Petr > Hi Xu, > > Reviewed-by: Petr Vorel<pvorel@suse.cz> > > Nice cleanup. > +1 for Cyril's note to TST_EXP_FD_SILENT(). > >> diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c >> +static void setup(void) >> +{ >> + pidfd_open_supported(); >> +} >> + >> static struct tst_test test = { >> - .min_kver = "5.3", >> + .setup = setup, >> .test_all = run, > why not just .setup = pidfd_open_supported; ? Yes. will remove this wrapper. Best Regards Yang Xu > > ... > >> diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c >> index 48470e5e1..f41afa2fc 100644 >> --- a/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c >> +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c > ... >> +static void setup(void) >> +{ >> + pidfd_open_supported(); >> +} >> + > And here as well. > > Kind regards, > Petr > >> static struct tst_test test = { >> - .min_kver = "5.3", >> + .setup = setup, >> .test_all = run, >> .forks_child = 1, >> .needs_checkpoints = 1,
diff --git a/include/lapi/pidfd_open.h b/include/lapi/pidfd_open.h index 9806c73d4..5cf10933e 100644 --- a/include/lapi/pidfd_open.h +++ b/include/lapi/pidfd_open.h @@ -9,11 +9,15 @@ #include <sys/syscall.h> #include <sys/types.h> - #include "lapi/syscalls.h" - #include "config.h" +static inline void pidfd_open_supported(void) +{ + /* allow the tests to fail early */ + tst_syscall(__NR_pidfd_open); +} + #ifndef HAVE_PIDFD_OPEN static inline int pidfd_open(pid_t pid, unsigned int flags) { diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c index f40e9b624..ed9b82637 100644 --- a/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c @@ -1,11 +1,15 @@ // SPDX-License-Identifier: GPL-2.0-or-later /* * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org> + */ + +/*\ + * [Description] * - * Description: * Basic pidfd_open() test: - * 1) Fetch the PID of the current process and try to get its file descriptor. - * 2) Check that the close-on-exec flag is set on the file descriptor. + * + * - Fetch the PID of the current process and try to get its file descriptor. + * - Check that the close-on-exec flag is set on the file descriptor. */ #include <unistd.h> @@ -17,10 +21,7 @@ static void run(void) { int flag; - TEST(pidfd_open(getpid(), 0)); - - if (TST_RET == -1) - tst_brk(TFAIL | TTERRNO, "pidfd_open(getpid(), 0) failed"); + TST_EXP_PID_SILENT(pidfd_open(getpid(), 0), "pidfd_open(getpid(), 0)"); flag = fcntl(TST_RET, F_GETFD); @@ -35,7 +36,12 @@ static void run(void) tst_res(TPASS, "pidfd_open(getpid(), 0) passed"); } +static void setup(void) +{ + pidfd_open_supported(); +} + static struct tst_test test = { - .min_kver = "5.3", + .setup = setup, .test_all = run, }; diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c index dc86cae7a..93a61a51d 100644 --- a/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c @@ -1,14 +1,21 @@ // SPDX-License-Identifier: GPL-2.0-or-later /* * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org> + */ + +/*\ + * [Description] + * + * Tests basic error handling of the pidfd_open syscall. * - * Description: - * Basic pidfd_open() test to test invalid arguments. + * - ESRCH the process specified by pid does not exist + * - EINVAL pid is not valid + * - EINVAL flags is not valid */ #include "tst_test.h" #include "lapi/pidfd_open.h" -pid_t expired_pid, my_pid, invalid_pid = -1; +static pid_t expired_pid, my_pid, invalid_pid = -1; static struct tcase { char *name; @@ -23,6 +30,7 @@ static struct tcase { static void setup(void) { + pidfd_open_supported(); expired_pid = tst_get_unused_pid(); my_pid = getpid(); } @@ -31,27 +39,11 @@ static void run(unsigned int n) { struct tcase *tc = &tcases[n]; - TEST(pidfd_open(*tc->pid, tc->flags)); - - if (TST_RET != -1) { - SAFE_CLOSE(TST_RET); - tst_res(TFAIL, "%s: pidfd_open succeeded unexpectedly (index: %d)", - tc->name, n); - return; - } - - if (tc->exp_errno != TST_ERR) { - tst_res(TFAIL | TTERRNO, "%s: pidfd_open() should fail with %s", - tc->name, tst_strerrno(tc->exp_errno)); - return; - } - - tst_res(TPASS | TTERRNO, "%s: pidfd_open() failed as expected", - tc->name); + TST_EXP_FAIL2(pidfd_open(*tc->pid, tc->flags), tc->exp_errno, + "pidfd_open with %s", tc->name); } static struct tst_test test = { - .min_kver = "5.3", .tcnt = ARRAY_SIZE(tcases), .test = run, .setup = setup, diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c index 48470e5e1..f41afa2fc 100644 --- a/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c @@ -1,8 +1,11 @@ // SPDX-License-Identifier: GPL-2.0-or-later /* * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org> + */ + +/*\ + * [Description] * - * Description: * This program opens the PID file descriptor of the child process created with * fork(). It then uses poll to monitor the file descriptor for process exit, as * indicated by an EPOLLIN event. @@ -27,11 +30,9 @@ static void run(void) exit(EXIT_SUCCESS); } - TEST(pidfd_open(pid, 0)); + TST_EXP_PID_SILENT(pidfd_open(pid, 0), "pidfd_open(%d, 0)", pid); fd = TST_RET; - if (fd == -1) - tst_brk(TFAIL | TTERRNO, "pidfd_open() failed"); TST_CHECKPOINT_WAKE(0); @@ -49,8 +50,13 @@ static void run(void) tst_res(TPASS, "pidfd_open() passed"); } +static void setup(void) +{ + pidfd_open_supported(); +} + static struct tst_test test = { - .min_kver = "5.3", + .setup = setup, .test_all = run, .forks_child = 1, .needs_checkpoints = 1,
1) make use of TST_EXP_FAIL2 or TST_EXP_PID_SILENT macros 2) remove min_kver check and use pidfd_open_supported() 3) Add docparse formatting Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> --- include/lapi/pidfd_open.h | 8 +++-- .../kernel/syscalls/pidfd_open/pidfd_open01.c | 22 +++++++----- .../kernel/syscalls/pidfd_open/pidfd_open02.c | 34 +++++++------------ .../kernel/syscalls/pidfd_open/pidfd_open03.c | 16 ++++++--- 4 files changed, 44 insertions(+), 36 deletions(-)