| Message ID | 1697698029-31949-1-git-send-email-xuyang2018.jy@fujitsu.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | swapon02: Simplify code, add copyright, modify docparse | expand |
Hi Ping Best Regards, Yang Xu >Simplify permission-related test code, making structures look simpler >Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> >--- >testcases/kernel/syscalls/swapon/swapon02.c | 57 +++++++-------------- >1 file changed, 18 insertions(+), 39 deletions(-) >diff --git a/testcases/kernel/syscalls/swapon/swapon02.c b/testcases/kernel/syscalls/swapon/swapon02.c >index d34c17a80..2c9e39986 100644 >--- a/testcases/kernel/syscalls/swapon/swapon02.c >+++ b/testcases/kernel/syscalls/swapon/swapon02.c
On 2023/10/19 14:47, Yang Xu wrote: > Simplify permission-related test code, making structures look simpler > > Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> > --- > testcases/kernel/syscalls/swapon/swapon02.c | 57 +++++++-------------- > 1 file changed, 18 insertions(+), 39 deletions(-) > > diff --git a/testcases/kernel/syscalls/swapon/swapon02.c b/testcases/kernel/syscalls/swapon/swapon02.c > index d34c17a80..2c9e39986 100644 > --- a/testcases/kernel/syscalls/swapon/swapon02.c > +++ b/testcases/kernel/syscalls/swapon/swapon02.c > @@ -1,17 +1,17 @@ > // SPDX-License-Identifier: GPL-2.0-or-later > - > /* > * Copyright (c) Wipro Technologies Ltd, 2002. All Rights Reserved. > + * Copyright (c) Linux Test Project, 2003-2023 > */ > > /*\ > * [Description] > * > * This test case checks whether swapon(2) system call returns > - * 1. ENOENT when the path does not exist > - * 2. EINVAL when the path exists but is invalid > - * 3. EPERM when user is not a superuser > - * 4. EBUSY when the specified path is already being used as a swap area > + * - ENOENT when the path does not exist. > + * - EINVAL when the path exists but is invalid. > + * - EPERM when user is not a superuser. > + * - EBUSY when the specified path is already being used as a swap area. > */ > > #include <errno.h> > @@ -21,36 +21,20 @@ > #include "lapi/syscalls.h" > #include "libswap.h" > > -static void setup01(void); > -static void cleanup01(void); > - > static uid_t nobody_uid; > static int do_swapoff; > > static struct tcase { > char *err_desc; > int exp_errno; > - char *exp_errval; > char *path; > - void (*setup)(void); > - void (*cleanup)(void); > } tcases[] = { > - {"Path does not exist", ENOENT, "ENOENT", "./doesnotexist", NULL, NULL}, > - {"Invalid path", EINVAL, "EINVAL", "./notswap", NULL, NULL}, > - {"Permission denied", EPERM, "EPERM", "./swapfile01", setup01, cleanup01}, > - {"File already used", EBUSY, "EBUSY", "./alreadyused", NULL, NULL}, > + {"Path does not exist", ENOENT, "./doesnotexist"}, > + {"Invalid path", EINVAL, "./notswap"}, > + {"Permission denied", EPERM, "./swapfile01"}, > + {"File already used", EBUSY, "./alreadyused"}, > }; > > -static void setup01(void) > -{ > - SAFE_SETEUID(nobody_uid); > -} > - > -static void cleanup01(void) > -{ > - SAFE_SETEUID(0); > -} > - > static void setup(void) > { > struct passwd *nobody; > @@ -79,24 +63,19 @@ void cleanup(void) Hi Yang It looks good to me. one minor hint: static void cleanup() should be better. Reviewed-by: Xiao Yang <yangx.jy@fujitsu.com> Best Regards, Xiao Yang > static void verify_swapon(unsigned int i) > { > struct tcase *tc = tcases + i; > - if (tc->setup) > - tc->setup(); > + if (tc->exp_errno == EPERM) > + SAFE_SETEUID(nobody_uid); > > - TEST(tst_syscall(__NR_swapon, tc->path, 0)); > + TST_EXP_FAIL(tst_syscall(__NR_swapon, tc->path, 0), tc->exp_errno, > + "swapon(2) fail with %s", tc->err_desc); > > - if (tc->cleanup) > - tc->cleanup(); > + if (tc->exp_errno == EPERM) > + SAFE_SETEUID(0); > > - if (TST_RET == -1 && TST_ERR == tc->exp_errno) { > - tst_res(TPASS, "swapon(2) expected failure;" > - " Got errno - %s : %s", > - tc->exp_errval, tc->err_desc); > - return; > + if (TST_RET != -1) { > + tst_res(TFAIL, "swapon(2) failed unexpectedly, expected: %s", > + tst_strerrno(tc->exp_errno)); > } > - > - tst_res(TFAIL, "swapon(2) failed to produce expected error:" > - " %d, errno: %s and got %d.", tc->exp_errno, > - tc->exp_errval, TST_ERR); > } > > static struct tst_test test = {
Hi, Xiao Yang >On 2023/10/19 14:47, Yang Xu wrote: >> Simplify permission-related test code, making structures look simpler >> >> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> >> --- >> testcases/kernel/syscalls/swapon/swapon02.c | 57 +++++++-------------- >> 1 file changed, 18 insertions(+), 39 deletions(-) >> >> diff --git a/testcases/kernel/syscalls/swapon/swapon02.c b/testcases/kernel/syscalls/swapon/swapon02.c >> index d34c17a80..2c9e39986 100644 >> --- a/testcases/kernel/syscalls/swapon/swapon02.c >> +++ b/testcases/kernel/syscalls/swapon/swapon02.c >> @@ -1,17 +1,17 @@ >> // SPDX-License-Identifier: GPL-2.0-or-later >> - >> /* >> * Copyright (c) Wipro Technologies Ltd, 2002. All Rights Reserved. >> + * Copyright (c) Linux Test Project, 2003-2023 >> */ >> >> /*\ >> * [Description] >> * >> * This test case checks whether swapon(2) system call returns >> - * 1. ENOENT when the path does not exist >> - * 2. EINVAL when the path exists but is invalid >> - * 3. EPERM when user is not a superuser >> - * 4. EBUSY when the specified path is already being used as a swap area >> + * - ENOENT when the path does not exist. >> + * - EINVAL when the path exists but is invalid. >> + * - EPERM when user is not a superuser. >> + * - EBUSY when the specified path is already being used as a swap area. >> */ >> >> #include <errno.h> >> @@ -21,36 +21,20 @@ >> #include "lapi/syscalls.h" >> #include "libswap.h" >> >> -static void setup01(void); >> -static void cleanup01(void); >> - ... >> - >> static void setup(void) >> { >> struct passwd *nobody; >> @@ -79,24 +63,19 @@ void cleanup(void) >Hi Yang >It looks good to me. one minor hint: >static void cleanup() should be better. >Reviewed-by: Xiao Yang <yangx.jy@fujitsu.com> >Best Regards, >Xiao Yang OK. I'll modify it. Best Regards, Yang Xu
diff --git a/testcases/kernel/syscalls/swapon/swapon02.c b/testcases/kernel/syscalls/swapon/swapon02.c index d34c17a80..2c9e39986 100644 --- a/testcases/kernel/syscalls/swapon/swapon02.c +++ b/testcases/kernel/syscalls/swapon/swapon02.c @@ -1,17 +1,17 @@ // SPDX-License-Identifier: GPL-2.0-or-later - /* * Copyright (c) Wipro Technologies Ltd, 2002. All Rights Reserved. + * Copyright (c) Linux Test Project, 2003-2023 */ /*\ * [Description] * * This test case checks whether swapon(2) system call returns - * 1. ENOENT when the path does not exist - * 2. EINVAL when the path exists but is invalid - * 3. EPERM when user is not a superuser - * 4. EBUSY when the specified path is already being used as a swap area + * - ENOENT when the path does not exist. + * - EINVAL when the path exists but is invalid. + * - EPERM when user is not a superuser. + * - EBUSY when the specified path is already being used as a swap area. */ #include <errno.h> @@ -21,36 +21,20 @@ #include "lapi/syscalls.h" #include "libswap.h" -static void setup01(void); -static void cleanup01(void); - static uid_t nobody_uid; static int do_swapoff; static struct tcase { char *err_desc; int exp_errno; - char *exp_errval; char *path; - void (*setup)(void); - void (*cleanup)(void); } tcases[] = { - {"Path does not exist", ENOENT, "ENOENT", "./doesnotexist", NULL, NULL}, - {"Invalid path", EINVAL, "EINVAL", "./notswap", NULL, NULL}, - {"Permission denied", EPERM, "EPERM", "./swapfile01", setup01, cleanup01}, - {"File already used", EBUSY, "EBUSY", "./alreadyused", NULL, NULL}, + {"Path does not exist", ENOENT, "./doesnotexist"}, + {"Invalid path", EINVAL, "./notswap"}, + {"Permission denied", EPERM, "./swapfile01"}, + {"File already used", EBUSY, "./alreadyused"}, }; -static void setup01(void) -{ - SAFE_SETEUID(nobody_uid); -} - -static void cleanup01(void) -{ - SAFE_SETEUID(0); -} - static void setup(void) { struct passwd *nobody; @@ -79,24 +63,19 @@ void cleanup(void) static void verify_swapon(unsigned int i) { struct tcase *tc = tcases + i; - if (tc->setup) - tc->setup(); + if (tc->exp_errno == EPERM) + SAFE_SETEUID(nobody_uid); - TEST(tst_syscall(__NR_swapon, tc->path, 0)); + TST_EXP_FAIL(tst_syscall(__NR_swapon, tc->path, 0), tc->exp_errno, + "swapon(2) fail with %s", tc->err_desc); - if (tc->cleanup) - tc->cleanup(); + if (tc->exp_errno == EPERM) + SAFE_SETEUID(0); - if (TST_RET == -1 && TST_ERR == tc->exp_errno) { - tst_res(TPASS, "swapon(2) expected failure;" - " Got errno - %s : %s", - tc->exp_errval, tc->err_desc); - return; + if (TST_RET != -1) { + tst_res(TFAIL, "swapon(2) failed unexpectedly, expected: %s", + tst_strerrno(tc->exp_errno)); } - - tst_res(TFAIL, "swapon(2) failed to produce expected error:" - " %d, errno: %s and got %d.", tc->exp_errno, - tc->exp_errval, TST_ERR); } static struct tst_test test = {
Simplify permission-related test code, making structures look simpler Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> --- testcases/kernel/syscalls/swapon/swapon02.c | 57 +++++++-------------- 1 file changed, 18 insertions(+), 39 deletions(-)