[LTP] [PATCH] swapon02: Simplify code, add copyright, modify docparse

[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 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 = {