[LTP] [PATCH] acct01: add EFAULT errno check

[路斐]

Hi Cyril
Thanks for reply.
In the beginning, replace ENOSYS with .needs_kconfigs is for fixing one of the `make check` warnings. Should I still split .needs_kconfigs and make check fixes to seperate patches? 

Best reguards.
Lufei

 
 
------------------ Original ------------------
From:  "Cyril Hrubis"<chrubis@suse.cz>;
Date:  Thu, Jul 4, 2024 01:29 PM
To:  "lufei"<lufei@uniontech.com>; 
Cc:  "ltp"<ltp@lists.linux.it>; 
Subject:  Re: [LTP] [PATCH] acct01: add EFAULT errno check

 

Hi!
> 1. add EFAULT errno check.
> 2. fix make check errors and warnings.

Can you please split the functional changes and make chek fixes into
separate tests?

Ideally each type of change should be put into a separate patch.

> Signed-off-by: lufei <lufei@uniontech.com>
> ---
>  testcases/kernel/syscalls/acct/acct01.c | 31 +++++++++++++++++--------
>  testcases/kernel/syscalls/acct/acct02.c |  2 +-
>  2 files changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/acct/acct01.c b/testcases/kernel/syscalls/acct/acct01.c
> index a05ed2ea9..ed1817bc5 100644
> --- a/testcases/kernel/syscalls/acct/acct01.c
> +++ b/testcases/kernel/syscalls/acct/acct01.c
> @@ -25,8 +25,7 @@
>  
>  #include "tst_test.h"
>  
> -#define DIR_MODE	(S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP| \
> -			 S_IXGRP|S_IROTH|S_IXOTH)
> +#define DIR_MODE	0755
>  #define FILE_EISDIR		"."
>  #define FILE_EACCESS		"/dev/null"
>  #define FILE_ENOENT		"/tmp/does/not/exist"
> @@ -34,6 +33,7 @@
>  #define FILE_TMPFILE		"./tmpfile"
>  #define FILE_ELOOP		"test_file_eloop1"
>  #define FILE_EROFS		"ro_mntpoint/file"
> +#define FILE_EFAULT		"/tmp/invalid/file/name"
>  
>  static struct passwd *ltpuser;
>  
> @@ -46,6 +46,7 @@ static char *file_eloop;
>  static char *file_enametoolong;
>  static char *file_erofs;
>  static char *file_null;
> +static char *file_efault;
>  
>  static void setup_euid(void)
>  {
> @@ -57,12 +58,22 @@ static void cleanup_euid(void)
>  	SAFE_SETEUID(0);
>  }
>  
> +static void setup_emem(void)
> +{
> +	file_efault = SAFE_MMAP(NULL, 1, PROT_NONE,
> +			MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
> +}
> +static void cleanup_emem(void)
> +{
> +	SAFE_MUNMAP(file_efault, 1);
> +}
> +
>  static struct test_case {
>  	char **filename;
>  	char *desc;
>  	int exp_errno;
> -	void (*setupfunc) ();
> -	void (*cleanfunc) ();
> +	void (*setupfunc)();
> +	void (*cleanfunc)();
>  } tcases[] = {
>  	{&file_eisdir,  FILE_EISDIR,  EISDIR,  NULL,   NULL},
>  	{&file_eaccess, FILE_EACCESS, EACCES,  NULL,   NULL},
> @@ -73,16 +84,13 @@ static struct test_case {
>  	{&file_eloop,   FILE_ELOOP,   ELOOP,        NULL, NULL},
>  	{&file_enametoolong, "aaaa...", ENAMETOOLONG, NULL, NULL},
>  	{&file_erofs,   FILE_EROFS,   EROFS,        NULL, NULL},
> +	{&file_efault,	FILE_EFAULT,  EFAULT,  setup_emem, cleanup_emem},
                         ^
			 This should actually describe the testcase so
			 it should be something as "Invalid address"

>  };
>  
>  static void setup(void)
>  {
>  	int fd;
>  
> -	TEST(acct(NULL));
> -	if (TST_RET == -1 && TST_ERR == ENOSYS)
> -		tst_brk(TCONF, "acct() system call isn't configured in kernel");
> -
>  	ltpuser = SAFE_GETPWNAM("nobody");
>  
>  	fd = SAFE_CREAT(FILE_TMPFILE, 0777);
> @@ -113,7 +121,7 @@ static void verify_acct(unsigned int nr)
>  		tcase->setupfunc();
>  
>  	TST_EXP_FAIL(acct(*tcase->filename), tcase->exp_errno,
> -	             "acct(%s)", tcase->desc);
> +			"acct(%s)", tcase->desc);
>  
>  	if (tcase->cleanfunc)
>  		tcase->cleanfunc();
> @@ -136,5 +144,8 @@ static struct tst_test test = {
>  		{&file_enametoolong, .size = PATH_MAX+2},
>  		{&file_erofs, .str = FILE_EROFS},
>  		{}
> -	}
> +	},
> +	.needs_kconfigs = (const char *[]) {
> +		"CONFIG_BSD_PROCESS_ACCT=y",
> +	},
>  };

And this is a different change that is not described in the patch
description. So this patch should be actually split into three patches,
one that adds errno check, one that adds needs_kconfigs and one that
fixes make check errors.

> diff --git a/testcases/kernel/syscalls/acct/acct02.c b/testcases/kernel/syscalls/acct/acct02.c
> index d3f3d9d04..74019f430 100644
> --- a/testcases/kernel/syscalls/acct/acct02.c
> +++ b/testcases/kernel/syscalls/acct/acct02.c
> @@ -186,7 +186,7 @@ static void run(void)
>  
>  		if (read_bytes != acct_size) {
>  			tst_res(TFAIL, "incomplete read %i bytes, expected %i",
> -			        read_bytes, acct_size);
> +					read_bytes, acct_size);
>  			goto exit;
>  		}
>  
> -- 
> 2.39.3
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz