[LTP] [PATCH] [v1] syscalls/symlinkat01: Convert to new API

[LTP] [PATCH] [v1] syscalls/symlinkat01: Convert to new API

[sujiaxun]

From: sujiauxn <sujiaxun@uniontech.com>

Signed-off-by: sujiauxn <sujiaxun@uniontech.com>
---
 .../kernel/syscalls/symlinkat/symlinkat01.c   | 198 ++++++------------
 1 file changed, 63 insertions(+), 135 deletions(-)

diff --git a/testcases/kernel/syscalls/symlinkat/symlinkat01.c b/testcases/kernel/syscalls/symlinkat/symlinkat01.c
index 8c9e148e9..beda359c1 100644
--- a/testcases/kernel/syscalls/symlinkat/symlinkat01.c
+++ b/testcases/kernel/syscalls/symlinkat/symlinkat01.c
@@ -1,35 +1,16 @@
-/******************************************************************************
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) International Business Machines  Corp., 2006
+ * 08/25/2006      Created first by Yi Yang <yyangcdl@cn.ibm.com>
+ */
+
+/*\
+ * [Description]
  *
- *   Copyright (c) International Business Machines  Corp., 2006
+ * This test case will verify basic function of symlinkat
+ * added by kernel 2.6.16 or up.
  *
- *   This program is free software;  you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation; either version 2 of the License, or
- *   (at your option) any later version.
- *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with this program;  if not, write to the Free Software
- *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- *
- * NAME
- *      symlinkat01.c
- *
- * DESCRIPTION
- *	This test case will verify basic function of symlinkat
- *	added by kernel 2.6.16 or up.
- *
- * Author
- *	Yi Yang <yyangcdl@cn.ibm.com>
- *
- * History
- *      08/25/2006      Created first by Yi Yang <yyangcdl@cn.ibm.com>
- *
- *****************************************************************************/
+ */

 #define _GNU_SOURCE

@@ -38,24 +19,16 @@
 #include <sys/time.h>
 #include <fcntl.h>
 #include <stdlib.h>
-#include <errno.h>
-#include <string.h>
 #include <signal.h>
-#include "test.h"
-#include "safe_macros.h"
+#include "tst_test.h"
 #include "lapi/syscalls.h"
+#include <stdio.h>

 #define MYRETCODE -999
 #ifndef AT_FDCWD
 #define AT_FDCWD -100
 #endif

-struct test_struct;
-static void setup();
-static void cleanup();
-static void setup_every_copy();
-static void mysymlinkat_test(struct test_struct *desc);
-
 #define TEST_DIR1 "olddir"
 #define TEST_DIR2 "newdir"
 #define TEST_DIR3 "deldir"
@@ -67,146 +40,104 @@ static char dpathname[256] = "%s/" TEST_DIR2 "/" TEST_FILE1;
 static int olddirfd, newdirfd = -1, cwd_fd = AT_FDCWD, stdinfd = 0, crapfd =
     -1, deldirfd;

-struct test_struct {
+struct test_case {
 	const char *oldfn;
 	int *newfd;
 	const char *newfn;
 	const char *referencefn1;
 	const char *referencefn2;
 	int expected_errno;
-} test_desc[] = {
+} tcases[] = {
 	/* relative paths */
 	{
 	"../" TEST_DIR1 "/" TEST_FILE1, &newdirfd, TEST_FILE1,
 		    TEST_DIR1 "/" TEST_FILE1, TEST_DIR2 "/" TEST_FILE1, 0},
-	    /* abs path at dst */
+    /* abs path at dst */
 	{
 	"../" TEST_DIR1 "/" TEST_FILE1, &newdirfd, dpathname,
 		    TEST_DIR1 "/" TEST_FILE1, TEST_DIR2 "/" TEST_FILE1, 0},
-	    /* relative paths to cwd */
+    /* relative paths to cwd */
 	{
 	"../" TEST_DIR1 "/" TEST_FILE1, &cwd_fd,
 		    TEST_DIR2 "/" TEST_FILE1, TEST_DIR1 "/" TEST_FILE1,
 		    TEST_DIR2 "/" TEST_FILE1, 0},
-	    /* abs path */
+    /* abs path */
 	{
 	"../" TEST_DIR1 "/" TEST_FILE1, &cwd_fd, dpathname,
 		    TEST_DIR1 "/" TEST_FILE1, TEST_DIR2 "/" TEST_FILE1, 0},
-	    /* relative paths to invalid */
+    /* relative paths to invalid */
 	{
 	"../" TEST_DIR1 "/" TEST_FILE1, &stdinfd,
 		    TEST_DIR2 "/" TEST_FILE1, 0, 0, ENOTDIR},
-	    /* abs path at dst */
+    /* abs path at dst */
 	{
 	"../" TEST_DIR1 "/" TEST_FILE1, &stdinfd, dpathname,
 		    TEST_DIR1 "/" TEST_FILE1, TEST_DIR2 "/" TEST_FILE1, 0},
-	    /* relative paths to crap */
+    /* relative paths to crap */
 	{
 	"../" TEST_DIR1 "/" TEST_FILE1, &crapfd,
 		    TEST_DIR2 "/" TEST_FILE1, 0, 0, EBADF},
-	    /* abs path at dst */
+    /* abs path at dst */
 	{
 	"../" TEST_DIR1 "/" TEST_FILE1, &crapfd, dpathname,
 		    TEST_DIR1 "/" TEST_FILE1, TEST_DIR2 "/" TEST_FILE1, 0},
-	    /* relative paths to deleted */
+    /* relative paths to deleted */
 	{
 	"../" TEST_DIR1 "/" TEST_FILE1, &deldirfd,
 		    TEST_DIR2 "/" TEST_FILE1, 0, 0, ENOENT},
-	    /* abs path at dst */
+    /* abs path at dst */
 	{
 	"../" TEST_DIR1 "/" TEST_FILE1, &deldirfd, dpathname,
-		    TEST_DIR1 "/" TEST_FILE1, TEST_DIR2 "/" TEST_FILE1, 0},
-	    /* fifo link */
-	    /*      { TEST_FIFO, &newdirfd, TEST_FILE1, TEST_DIR1"/"TEST_FIFO, TEST_DIR2"/"TEST_FILE1, 0 }, */
+		    TEST_DIR1 "/" TEST_FILE1, TEST_DIR2 "/" TEST_FILE1, 0}
 };

-char *TCID = "symlinkat01";
-int TST_TOTAL = sizeof(test_desc) / sizeof(*test_desc);
-
-static int mysymlinkat(const char *oldfilename,
-		       int newdirfd, const char *newfilename)
-{
-	return ltp_syscall(__NR_symlinkat, oldfilename, newdirfd, newfilename);
-}
-
-int main(int ac, char **av)
-{
-	int lc;
-	int i;
-
-	/* Disable test if the version of the kernel is less than 2.6.16 */
-	if ((tst_kvercmp(2, 6, 16)) < 0) {
-		tst_resm(TWARN, "This test can only run on kernels that are ");
-		tst_resm(TWARN, "2.6.16 and higher");
-		exit(0);
-	}
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-
-		tst_count = 0;
-
-		for (i = 0; i < TST_TOTAL; i++) {
-			setup_every_copy();
-			mysymlinkat_test(&test_desc[i]);
-
-		}
-
-	}
-
-	cleanup();
-	tst_exit();
-}
-
 static void setup_every_copy(void)
 {
 	close(newdirfd);
 	unlink(dpathname);
 	rmdir(TEST_DIR2);

-	SAFE_MKDIR(cleanup, TEST_DIR2, 0700);
-	newdirfd = SAFE_OPEN(cleanup, TEST_DIR2, O_DIRECTORY);
+	SAFE_MKDIR(TEST_DIR2, 0700);
+	newdirfd = SAFE_OPEN(TEST_DIR2, O_DIRECTORY);
 }

-static void mysymlinkat_test(struct test_struct *desc)
+static void run_test(unsigned int nr)
 {
 	int fd;
+	struct test_case *tc = &tcases[nr];
+
+	setup_every_copy();

-	TEST(mysymlinkat(desc->oldfn, *desc->newfd, desc->newfn));
-
-	/* check return code */
-	if (TEST_ERRNO == desc->expected_errno) {
-		if (TEST_RETURN == 0 && desc->referencefn1 != NULL) {
+	TEST(tst_syscall(__NR_symlinkat, tc->oldfn, *tc->newfd, tc->newfn));
+	if (TST_ERR == tc->expected_errno) {
+		if (TST_RET == 0 && tc->referencefn1 != NULL) {
 			int tnum = rand(), vnum = ~tnum;

-			fd = SAFE_OPEN(cleanup, desc->referencefn1, O_RDWR);
-			SAFE_WRITE(cleanup, 1, fd, &tnum, sizeof(tnum));
-			SAFE_CLOSE(cleanup, fd);
+			fd = SAFE_OPEN(tc->referencefn1, O_RDWR);
+			SAFE_WRITE(1, fd, &tnum, sizeof(tnum));
+			SAFE_CLOSE(fd);

-			fd = SAFE_OPEN(cleanup, desc->referencefn2, O_RDONLY);
-			SAFE_READ(cleanup, 1, fd, &vnum, sizeof(vnum));
-			SAFE_CLOSE(cleanup, fd);
+			fd = SAFE_OPEN(tc->referencefn2, O_RDONLY);
+			SAFE_READ(1, fd, &vnum, sizeof(vnum));
+			SAFE_CLOSE(fd);

 			if (tnum == vnum)
-				tst_resm(TPASS, "Test passed");
+				tst_res(TPASS, "Test passed");
 			else
-				tst_resm(TFAIL,
+				tst_res(TFAIL,
 					 "The link file's content isn't as same as the original file's "
 					 "although symlinkat returned 0");
 		} else {
-			tst_resm(TPASS,
+			tst_res(TPASS,
 				 "symlinkat() returned the expected  errno %d: %s",
-				 TEST_ERRNO, strerror(TEST_ERRNO));
+				 TST_ERR, strerror(TST_ERR));
 		}
 	} else {
-		tst_resm(TFAIL,
-			 TEST_RETURN ==
+		tst_res(TFAIL,
+			 TST_RET ==
 			 0 ? "symlinkat() surprisingly succeeded" :
-			 "symlinkat() Failed, errno=%d : %s", TEST_ERRNO,
-			 strerror(TEST_ERRNO));
+			 "symlinkat() Failed, errno=%d : %s", TST_ERR,
+			 strerror(TST_ERR));
 	}
 }

@@ -214,27 +145,24 @@ static void setup(void)
 {
 	char *tmp;
 	int fd;
-
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
-
-	tst_tmpdir();
-
-	SAFE_MKDIR(cleanup, TEST_DIR1, 0700);
-	SAFE_MKDIR(cleanup, TEST_DIR3, 0700);
-	olddirfd = SAFE_OPEN(cleanup, TEST_DIR1, O_DIRECTORY);
-	deldirfd = SAFE_OPEN(cleanup, TEST_DIR3, O_DIRECTORY);
-	SAFE_RMDIR(cleanup, TEST_DIR3);
-	fd = SAFE_OPEN(cleanup, TEST_DIR1 "/" TEST_FILE1, O_CREAT | O_EXCL, 0600);
-	SAFE_CLOSE(cleanup, fd);
+
+	SAFE_MKDIR(TEST_DIR1, 0700);
+	SAFE_MKDIR(TEST_DIR3, 0700);
+	olddirfd = SAFE_OPEN(TEST_DIR1, O_DIRECTORY);
+	deldirfd = SAFE_OPEN(TEST_DIR3, O_DIRECTORY);
+	SAFE_RMDIR(TEST_DIR3);
+	fd = SAFE_OPEN(TEST_DIR1 "/" TEST_FILE1, O_CREAT | O_EXCL, 0600);
+	SAFE_CLOSE(fd);

 	/* gratuitous memory leak here */
 	tmp = strdup(dpathname);
 	snprintf(dpathname, sizeof(dpathname), tmp, get_current_dir_name());
-
-	TEST_PAUSE;
 }

-static void cleanup(void)
-{
-	tst_rmdir();
-}
+static struct tst_test test = {
+    .min_kver = "2.6.16",
+	.setup = setup,
+    .tcnt = ARRAY_SIZE(tcases),
+    .test = run_test,
+    .needs_tmpdir = 1,
+};
--
2.20.1




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

Re: [LTP] [PATCH] [v1] syscalls/symlinkat01: Convert to new API

[Cyril Hrubis]

Hi!
> -static void mysymlinkat_test(struct test_struct *desc)
> +static void run_test(unsigned int nr)
>  {
>  	int fd;
> +	struct test_case *tc = &tcases[nr];
> +
> +	setup_every_copy();
> 
> -	TEST(mysymlinkat(desc->oldfn, *desc->newfd, desc->newfn));
> -
> -	/* check return code */
> -	if (TEST_ERRNO == desc->expected_errno) {
> -		if (TEST_RETURN == 0 && desc->referencefn1 != NULL) {
> +	TEST(tst_syscall(__NR_symlinkat, tc->oldfn, *tc->newfd, tc->newfn));
> +	if (TST_ERR == tc->expected_errno) {
> +		if (TST_RET == 0 && tc->referencefn1 != NULL) {
>  			int tnum = rand(), vnum = ~tnum;
> 
> -			fd = SAFE_OPEN(cleanup, desc->referencefn1, O_RDWR);
> -			SAFE_WRITE(cleanup, 1, fd, &tnum, sizeof(tnum));
> -			SAFE_CLOSE(cleanup, fd);
> +			fd = SAFE_OPEN(tc->referencefn1, O_RDWR);
> +			SAFE_WRITE(1, fd, &tnum, sizeof(tnum));
> +			SAFE_CLOSE(fd);
> 
> -			fd = SAFE_OPEN(cleanup, desc->referencefn2, O_RDONLY);
> -			SAFE_READ(cleanup, 1, fd, &vnum, sizeof(vnum));
> -			SAFE_CLOSE(cleanup, fd);
> +			fd = SAFE_OPEN(tc->referencefn2, O_RDONLY);
> +			SAFE_READ(1, fd, &vnum, sizeof(vnum));
> +			SAFE_CLOSE(fd);
> 
>  			if (tnum == vnum)
> -				tst_resm(TPASS, "Test passed");
> +				tst_res(TPASS, "Test passed");
>  			else
> -				tst_resm(TFAIL,
> +				tst_res(TFAIL,
>  					 "The link file's content isn't as same as the original file's "
>  					 "although symlinkat returned 0");
>  		} else {
> -			tst_resm(TPASS,
> +			tst_res(TPASS,
>  				 "symlinkat() returned the expected  errno %d: %s",
> -				 TEST_ERRNO, strerror(TEST_ERRNO));
> +				 TST_ERR, strerror(TST_ERR));
>  		}

I think that the test would be much clearer to read if we split it into
two testcases, one that checks for successfull symlink creating and one
that checks for errors. Then we could easily use the TST_EXP_FAIL()
macros etc.

>  	} else {
> -		tst_resm(TFAIL,
> -			 TEST_RETURN ==
> +		tst_res(TFAIL,
> +			 TST_RET ==
>  			 0 ? "symlinkat() surprisingly succeeded" :
> -			 "symlinkat() Failed, errno=%d : %s", TEST_ERRNO,
> -			 strerror(TEST_ERRNO));
> +			 "symlinkat() Failed, errno=%d : %s", TST_ERR,
> +			 strerror(TST_ERR));
>  	}
>  }
> 
> @@ -214,27 +145,24 @@ static void setup(void)
>  {
>  	char *tmp;
>  	int fd;
> -
> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
> -
> -	tst_tmpdir();
> -
> -	SAFE_MKDIR(cleanup, TEST_DIR1, 0700);
> -	SAFE_MKDIR(cleanup, TEST_DIR3, 0700);
> -	olddirfd = SAFE_OPEN(cleanup, TEST_DIR1, O_DIRECTORY);
> -	deldirfd = SAFE_OPEN(cleanup, TEST_DIR3, O_DIRECTORY);
> -	SAFE_RMDIR(cleanup, TEST_DIR3);
> -	fd = SAFE_OPEN(cleanup, TEST_DIR1 "/" TEST_FILE1, O_CREAT | O_EXCL, 0600);
> -	SAFE_CLOSE(cleanup, fd);
> +
> +	SAFE_MKDIR(TEST_DIR1, 0700);
> +	SAFE_MKDIR(TEST_DIR3, 0700);
> +	olddirfd = SAFE_OPEN(TEST_DIR1, O_DIRECTORY);
> +	deldirfd = SAFE_OPEN(TEST_DIR3, O_DIRECTORY);
> +	SAFE_RMDIR(TEST_DIR3);
> +	fd = SAFE_OPEN(TEST_DIR1 "/" TEST_FILE1, O_CREAT | O_EXCL, 0600);
> +	SAFE_CLOSE(fd);
> 
>  	/* gratuitous memory leak here */
>  	tmp = strdup(dpathname);
>  	snprintf(dpathname, sizeof(dpathname), tmp, get_current_dir_name());

Can we fix this mess as well? There is no point actually in having the
format in a variable like this we can easily instead do:

	char *pwd;

	pwd = get_current_dir_name();
	snprintf(dpathname, sizeof(dpahtname), "%s/" TESTDIR2 "/" TEST_FILE1, pwd);
	free(pwd);

And that fixes two memory leaks we had there.

> -
> -	TEST_PAUSE;
>  }
> 
> -static void cleanup(void)
> -{
> -	tst_rmdir();
> -}
> +static struct tst_test test = {
> +    .min_kver = "2.6.16",
> +	.setup = setup,
> +    .tcnt = ARRAY_SIZE(tcases),
> +    .test = run_test,
> +    .needs_tmpdir = 1,
> +};

Other than that there are a few whitespaces wrong, see output of
'make check'

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH] [v1] syscalls/symlinkat01: Convert to new API

[sujiaxun]

在 2021/12/14 上午12:05, Cyril Hrubis 写道:
> Hi!
>> -static void mysymlinkat_test(struct test_struct *desc)
>> +static void run_test(unsigned int nr)
>>   {
>>   	int fd;
>> +	struct test_case *tc = &tcases[nr];
>> +
>> +	setup_every_copy();
>>
>> -	TEST(mysymlinkat(desc->oldfn, *desc->newfd, desc->newfn));
>> -
>> -	/* check return code */
>> -	if (TEST_ERRNO == desc->expected_errno) {
>> -		if (TEST_RETURN == 0 && desc->referencefn1 != NULL) {
>> +	TEST(tst_syscall(__NR_symlinkat, tc->oldfn, *tc->newfd, tc->newfn));
>> +	if (TST_ERR == tc->expected_errno) {
>> +		if (TST_RET == 0 && tc->referencefn1 != NULL) {
>>   			int tnum = rand(), vnum = ~tnum;
>>
>> -			fd = SAFE_OPEN(cleanup, desc->referencefn1, O_RDWR);
>> -			SAFE_WRITE(cleanup, 1, fd, &tnum, sizeof(tnum));
>> -			SAFE_CLOSE(cleanup, fd);
>> +			fd = SAFE_OPEN(tc->referencefn1, O_RDWR);
>> +			SAFE_WRITE(1, fd, &tnum, sizeof(tnum));
>> +			SAFE_CLOSE(fd);
>>
>> -			fd = SAFE_OPEN(cleanup, desc->referencefn2, O_RDONLY);
>> -			SAFE_READ(cleanup, 1, fd, &vnum, sizeof(vnum));
>> -			SAFE_CLOSE(cleanup, fd);
>> +			fd = SAFE_OPEN(tc->referencefn2, O_RDONLY);
>> +			SAFE_READ(1, fd, &vnum, sizeof(vnum));
>> +			SAFE_CLOSE(fd);
>>
>>   			if (tnum == vnum)
>> -				tst_resm(TPASS, "Test passed");
>> +				tst_res(TPASS, "Test passed");
>>   			else
>> -				tst_resm(TFAIL,
>> +				tst_res(TFAIL,
>>   					 "The link file's content isn't as same as the original file's "
>>   					 "although symlinkat returned 0");
>>   		} else {
>> -			tst_resm(TPASS,
>> +			tst_res(TPASS,
>>   				 "symlinkat() returned the expected  errno %d: %s",
>> -				 TEST_ERRNO, strerror(TEST_ERRNO));
>> +				 TST_ERR, strerror(TST_ERR));
>>   		}
> 
> I think that the test would be much clearer to read if we split it into
> two testcases, one that checks for successfull symlink creating and one
> that checks for errors. Then we could easily use the TST_EXP_FAIL()
> macros etc.
> 
>>   	} else {
>> -		tst_resm(TFAIL,
>> -			 TEST_RETURN ==
>> +		tst_res(TFAIL,
>> +			 TST_RET ==
>>   			 0 ? "symlinkat() surprisingly succeeded" :
>> -			 "symlinkat() Failed, errno=%d : %s", TEST_ERRNO,
>> -			 strerror(TEST_ERRNO));
>> +			 "symlinkat() Failed, errno=%d : %s", TST_ERR,
>> +			 strerror(TST_ERR));
>>   	}
>>   }
>>
>> @@ -214,27 +145,24 @@ static void setup(void)
>>   {
>>   	char *tmp;
>>   	int fd;
>> -
>> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
>> -
>> -	tst_tmpdir();
>> -
>> -	SAFE_MKDIR(cleanup, TEST_DIR1, 0700);
>> -	SAFE_MKDIR(cleanup, TEST_DIR3, 0700);
>> -	olddirfd = SAFE_OPEN(cleanup, TEST_DIR1, O_DIRECTORY);
>> -	deldirfd = SAFE_OPEN(cleanup, TEST_DIR3, O_DIRECTORY);
>> -	SAFE_RMDIR(cleanup, TEST_DIR3);
>> -	fd = SAFE_OPEN(cleanup, TEST_DIR1 "/" TEST_FILE1, O_CREAT | O_EXCL, 0600);
>> -	SAFE_CLOSE(cleanup, fd);
>> +
>> +	SAFE_MKDIR(TEST_DIR1, 0700);
>> +	SAFE_MKDIR(TEST_DIR3, 0700);
>> +	olddirfd = SAFE_OPEN(TEST_DIR1, O_DIRECTORY);
>> +	deldirfd = SAFE_OPEN(TEST_DIR3, O_DIRECTORY);
>> +	SAFE_RMDIR(TEST_DIR3);
>> +	fd = SAFE_OPEN(TEST_DIR1 "/" TEST_FILE1, O_CREAT | O_EXCL, 0600);
>> +	SAFE_CLOSE(fd);
>>
>>   	/* gratuitous memory leak here */
>>   	tmp = strdup(dpathname);
>>   	snprintf(dpathname, sizeof(dpathname), tmp, get_current_dir_name());
> 
> Can we fix this mess as well? There is no point actually in having the
> format in a variable like this we can easily instead do:
> 
> 	char *pwd;
> 
> 	pwd = get_current_dir_name();
> 	snprintf(dpathname, sizeof(dpahtname), "%s/" TESTDIR2 "/" TEST_FILE1, pwd);
> 	free(pwd);
> 
> And that fixes two memory leaks we had there.
> 
>> -
>> -	TEST_PAUSE;
>>   }
>>
>> -static void cleanup(void)
>> -{
>> -	tst_rmdir();
>> -}
>> +static struct tst_test test = {
>> +    .min_kver = "2.6.16",
>> +	.setup = setup,
>> +    .tcnt = ARRAY_SIZE(tcases),
>> +    .test = run_test,
>> +    .needs_tmpdir = 1,
>> +};
> 
> Other than that there are a few whitespaces wrong, see output of
> 'make check'
> 
Thank you, I will resubmit after making changes.



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