[LTP] [PATCH] fchownat/fchownat01.c: cleanup

[LTP] [PATCH] fchownat/fchownat01.c: cleanup

[zenglg.jy]

cleanup of fchownat01.c

Signed-off-by: Zeng Linggang <zenglg.jy@cn.fujitsu.com>
---
 testcases/kernel/syscalls/fchownat/fchownat01.c | 209 ++++++++----------------
 1 file changed, 69 insertions(+), 140 deletions(-)

diff --git a/testcases/kernel/syscalls/fchownat/fchownat01.c b/testcases/kernel/syscalls/fchownat/fchownat01.c
index 8a81f41..b46a721 100644
--- a/testcases/kernel/syscalls/fchownat/fchownat01.c
+++ b/testcases/kernel/syscalls/fchownat/fchownat01.c
@@ -13,8 +13,8 @@
  *   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
+ *   along with this program;  if not, write to the Free Software Foundation,
+ *   Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  *
  * NAME
  *      fchownat01.c
@@ -55,89 +55,60 @@
 #include <signal.h>
 #include "test.h"
 #include "usctest.h"
-#include "linux_syscall_numbers.h"
-
-#define TEST_CASES 6
-#ifndef AT_FDCWD
-#define AT_FDCWD -100
-#endif
-void setup();
-void cleanup();
-void setup_every_copy();
+#include "safe_macros.h"
+
+static void setup(void);
+static void cleanup(void);
+
+static uid_t uid;
+static gid_t gid;
+static char pathname[PATH_MAX+1];
+static char testfile[PATH_MAX+1];
+static char testfile2[PATH_MAX+1];
+static char testfile3[PATH_MAX+1];
+static int dirfd;
+static int fd;
+static int no_fd;
+static int cu_fd;
+
+static struct test_case_t {
+	int ret;
+	int flag;
+	int *fds;
+	char *filenames;
+} test_cases[] = {
+	{0, 0, &dirfd, testfile},
+	{0, 0, &dirfd, testfile2},
+	{ENOTDIR, 0, &fd, testfile},
+	{EBADF, 0, &no_fd, testfile},
+	{EINVAL, 9999, &dirfd, testfile},
+	{0, 0, &cu_fd, testfile},
+};
 
 char *TCID = "fchownat01";
-int TST_TOTAL = TEST_CASES;
-char pathname[256];
-char testfile[256];
-char testfile2[256];
-char testfile3[256];
-int dirfd, fd, ret;
-int fds[TEST_CASES];
-char *filenames[TEST_CASES];
-int expected_errno[TEST_CASES] = { 0, 0, ENOTDIR, EBADF, EINVAL, 0 };
-int flags[TEST_CASES] = { 0, 0, 0, 0, 9999, 0 };
-
-uid_t uid;
-gid_t gid;
-
-int myfchownat(int dirfd, const char *filename, uid_t owner, gid_t group,
-	       int flags)
-{
-	return ltp_syscall(__NR_fchownat, dirfd, filename, owner, group, flags);
-}
+int TST_TOTAL = ARRAY_SIZE(test_cases);
 
 int main(int ac, char **av)
 {
 	int lc;
-	char *msg;
 	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);
-	}
-
-	/***************************************************************
-	 * parse standard options
-	 ***************************************************************/
-	if ((msg = parse_opts(ac, av, NULL, NULL)) != NULL)
-		tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);
-
-	/***************************************************************
-	 * perform global setup for test
-	 ***************************************************************/
 	setup();
 
-	/***************************************************************
-	 * check looping state if -c option given
-	 ***************************************************************/
 	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		setup_every_copy();
 
 		tst_count = 0;
 
-		/*
-		 * Call fchownat
-		 */
 		for (i = 0; i < TST_TOTAL; i++) {
-			TEST(myfchownat
-			     (fds[i], filenames[i], uid, gid, flags[i]));
-
-			/* check return code */
-			if (TEST_ERRNO == expected_errno[i]) {
-
-				/***************************************************************
-				 * only perform functional verification if flag set (-f not given)
-				 ***************************************************************/
-				if (STD_FUNCTIONAL_TEST) {
-					/* No Verification test, yet... */
-					tst_resm(TPASS,
-						 "fchownat() returned the expected  errno %d: %s",
-						 TEST_ERRNO,
-						 strerror(TEST_ERRNO));
-				}
+			TEST(fchownat(*test_cases[i].fds,
+				      test_cases[i].filenames, uid, gid,
+				      test_cases[i].flag));
+
+			if (TEST_ERRNO == test_cases[i].ret) {
+				tst_resm(TPASS,
+					 "fchownat() returned the expected "
+					 "errno %d: %s", TEST_ERRNO,
+					 strerror(TEST_ERRNO));
 			} else {
 				TEST_ERROR_LOG(TEST_ERRNO);
 				tst_resm(TFAIL,
@@ -148,97 +119,55 @@ int main(int ac, char **av)
 
 	}
 
-	/***************************************************************
-	 * cleanup and exit
-	 ***************************************************************/
 	cleanup();
 
-	return (0);
+	tst_exit();
 }
 
-void setup_every_copy()
+void setup()
 {
-	/* Initialize test dir and file names */
+	uid = geteuid();
+	gid = getegid();
+
+	tst_sig(NOFORK, DEF_HANDLER, cleanup);
+
+	TEST_PAUSE;
+
 	sprintf(pathname, "fchownattestdir%d", getpid());
-	sprintf(testfile, "fchownattestfile%d.txt", getpid());
-	sprintf(testfile2, "/tmp/fchownattestfile%d.txt", getpid());
-	sprintf(testfile3, "fchownattestdir%d/fchownattestfile%d.txt", getpid(),
-		getpid());
 
-	ret = mkdir(pathname, 0700);
-	if (ret < 0) {
-		perror("mkdir: ");
-		exit(-1);
-	}
+	SAFE_MKDIR(cleanup, pathname, 0700);
 
-	dirfd = open(pathname, O_DIRECTORY);
-	if (dirfd < 0) {
-		perror("open: ");
-		exit(-1);
-	}
+	dirfd = SAFE_OPEN(cleanup, pathname, O_DIRECTORY);
 
-	fd = open(testfile, O_CREAT | O_RDWR, 0600);
-	if (fd < 0) {
-		perror("open: ");
-		exit(-1);
-	}
+	sprintf(testfile, "fchownattestfile%d.txt", getpid());
 
-	fd = open(testfile2, O_CREAT | O_RDWR, 0600);
-	if (fd < 0) {
-		perror("open: ");
-		exit(-1);
-	}
+	SAFE_OPEN(cleanup, testfile, O_CREAT | O_RDWR, 0600);
 
-	fd = open(testfile3, O_CREAT | O_RDWR, 0600);
-	if (fd < 0) {
-		perror("open: ");
-		exit(-1);
-	}
+	sprintf(testfile2, "/tmp/fchownattestfile%d.txt", getpid());
 
-	fds[0] = fds[1] = fds[4] = dirfd;
-	fds[2] = fd;
-	fds[3] = 100;
-	fds[5] = AT_FDCWD;
+	SAFE_OPEN(cleanup, testfile2, O_CREAT | O_RDWR, 0600);
 
-	filenames[0] = filenames[2] = filenames[3] = filenames[4] =
-	    filenames[5] = testfile;
-	filenames[1] = testfile2;
-}
+	sprintf(testfile3, "fchownattestdir%d/fchownattestfile%d.txt", getpid(),
+		getpid());
 
-/***************************************************************
- * setup() - performs all ONE TIME setup for this test.
- ***************************************************************/
-void setup()
-{
-	/* Set uid and gid */
-	uid = geteuid();
-	gid = getegid();
+	fd = SAFE_OPEN(cleanup, testfile3, O_CREAT | O_RDWR, 0600);
 
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
+	no_fd = 100;
 
-	TEST_PAUSE;
+	cu_fd = AT_FDCWD;
 }
 
-/***************************************************************
- * cleanup() - performs all ONE TIME cleanup for this test at
- *		completion or premature exit.
- ***************************************************************/
 void cleanup()
 {
-	/* Remove them */
-	char tmppathname[256];
-	strcpy(tmppathname, pathname);
-
-	close(fd);
-	unlink(testfile);
-	unlink(testfile2);
-	unlink(testfile3);
+	SAFE_CLOSE(NULL, fd);
+
+	SAFE_UNLINK(NULL, testfile);
+
+	SAFE_UNLINK(NULL, testfile2);
+
+	SAFE_UNLINK(NULL, testfile3);
+
 	rmdir(pathname);
 
-	/*
-	 * print timing stats if that option was specified.
-	 * print errno log if that option was specified.
-	 */
 	TEST_CLEANUP;
-
 }
-- 
1.8.2.1




------------------------------------------------------------------------------
Shape the Mobile Experience: Free Subscription
Software experts and developers: Be at the forefront of tech innovation.
Intel(R) Software Adrenaline delivers strategic insight and game-changing 
conversations that shape the rapidly evolving mobile landscape. Sign up now. 
http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] fchownat/fchownat01.c: cleanup

[chrubis]

Hi!
> -uid_t uid;
> -gid_t gid;
> -
> -int myfchownat(int dirfd, const char *filename, uid_t owner, gid_t group,
> -	       int flags)
> -{
> -	return ltp_syscall(__NR_fchownat, dirfd, filename, owner, group, flags);
> -}

Hmm, glibc 2.4 (first one with fchownat() support) has been released in
2006. I wonder if seven years is long enough and if we can get rid of
the wrapper now... Moreover I'm not sure what is the status of
alternative libc implementations.

There still may be enterprise distributions that still have older glibc
than that. Anybody out there?

> +int TST_TOTAL = ARRAY_SIZE(test_cases);
>  
>  int main(int ac, char **av)
>  {
>  	int lc;
> -	char *msg;
>  	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);
> -	}

This should not be removed but rather moved to the first lines of
setup() function and the TWARN should be changed to TCONF as:

tst_brkm(TCONF, NULL, "This test needs kernel 2.6.16 or newer"

> -	/***************************************************************
> -	 * parse standard options
> -	 ***************************************************************/
> -	if ((msg = parse_opts(ac, av, NULL, NULL)) != NULL)
> -		tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);

Do not remove the option parsing code.

> -
> -	/***************************************************************
> -	 * perform global setup for test
> -	 ***************************************************************/
>  	setup();
>  
> -	/***************************************************************
> -	 * check looping state if -c option given
> -	 ***************************************************************/
>  	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -		setup_every_copy();
>  
>  		tst_count = 0;
>  
> -		/*
> -		 * Call fchownat
> -		 */
>  		for (i = 0; i < TST_TOTAL; i++) {
> -			TEST(myfchownat
> -			     (fds[i], filenames[i], uid, gid, flags[i]));
> -
> -			/* check return code */
> -			if (TEST_ERRNO == expected_errno[i]) {
> -
> -				/***************************************************************
> -				 * only perform functional verification if flag set (-f not given)
> -				 ***************************************************************/
> -				if (STD_FUNCTIONAL_TEST) {
> -					/* No Verification test, yet... */
> -					tst_resm(TPASS,
> -						 "fchownat() returned the expected  errno %d: %s",
> -						 TEST_ERRNO,
> -						 strerror(TEST_ERRNO));
> -				}
> +			TEST(fchownat(*test_cases[i].fds,
> +				      test_cases[i].filenames, uid, gid,
> +				      test_cases[i].flag));
> +
> +			if (TEST_ERRNO == test_cases[i].ret) {
> +				tst_resm(TPASS,
> +					 "fchownat() returned the expected "
> +					 "errno %d: %s", TEST_ERRNO,
> +					 strerror(TEST_ERRNO));
>  			} else {
>  				TEST_ERROR_LOG(TEST_ERRNO);
>  				tst_resm(TFAIL,
> @@ -148,97 +119,55 @@ int main(int ac, char **av)
>  
>  	}
>  
> -	/***************************************************************
> -	 * cleanup and exit
> -	 ***************************************************************/
>  	cleanup();
>  
> -	return (0);
> +	tst_exit();
>  }
>  
> -void setup_every_copy()
> +void setup()

static void setup(void) here please.

>  {
> -	/* Initialize test dir and file names */
> +	uid = geteuid();
> +	gid = getegid();
> +
> +	tst_sig(NOFORK, DEF_HANDLER, cleanup);
> +
> +	TEST_PAUSE;
> +
>  	sprintf(pathname, "fchownattestdir%d", getpid());
> -	sprintf(testfile, "fchownattestfile%d.txt", getpid());
> -	sprintf(testfile2, "/tmp/fchownattestfile%d.txt", getpid());
> -	sprintf(testfile3, "fchownattestdir%d/fchownattestfile%d.txt", getpid(),
> -		getpid());
>  
> -	ret = mkdir(pathname, 0700);
> -	if (ret < 0) {
> -		perror("mkdir: ");
> -		exit(-1);
> -	}
> +	SAFE_MKDIR(cleanup, pathname, 0700);

Make use of tst_tmpdir() here instead. And also drop the pid from the
test file names as the directory made by tst_tmpdir() is unique
allready.

> -	dirfd = open(pathname, O_DIRECTORY);
> -	if (dirfd < 0) {
> -		perror("open: ");
> -		exit(-1);
> -	}
> +	dirfd = SAFE_OPEN(cleanup, pathname, O_DIRECTORY);
>  
> -	fd = open(testfile, O_CREAT | O_RDWR, 0600);
> -	if (fd < 0) {
> -		perror("open: ");
> -		exit(-1);
> -	}
> +	sprintf(testfile, "fchownattestfile%d.txt", getpid());
>  
> -	fd = open(testfile2, O_CREAT | O_RDWR, 0600);
> -	if (fd < 0) {
> -		perror("open: ");
> -		exit(-1);
> -	}
> +	SAFE_OPEN(cleanup, testfile, O_CREAT | O_RDWR, 0600);
>  
> -	fd = open(testfile3, O_CREAT | O_RDWR, 0600);
> -	if (fd < 0) {
> -		perror("open: ");
> -		exit(-1);
> -	}
> +	sprintf(testfile2, "/tmp/fchownattestfile%d.txt", getpid());
>  
> -	fds[0] = fds[1] = fds[4] = dirfd;
> -	fds[2] = fd;
> -	fds[3] = 100;
> -	fds[5] = AT_FDCWD;
> +	SAFE_OPEN(cleanup, testfile2, O_CREAT | O_RDWR, 0600);
>  
> -	filenames[0] = filenames[2] = filenames[3] = filenames[4] =
> -	    filenames[5] = testfile;
> -	filenames[1] = testfile2;
> -}
> +	sprintf(testfile3, "fchownattestdir%d/fchownattestfile%d.txt", getpid(),
> +		getpid());
>  
> -/***************************************************************
> - * setup() - performs all ONE TIME setup for this test.
> - ***************************************************************/
> -void setup()
> -{
> -	/* Set uid and gid */
> -	uid = geteuid();
> -	gid = getegid();
> +	fd = SAFE_OPEN(cleanup, testfile3, O_CREAT | O_RDWR, 0600);
>  
> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
> +	no_fd = 100;
>  
> -	TEST_PAUSE;
> +	cu_fd = AT_FDCWD;
>  }
>  
> -/***************************************************************
> - * cleanup() - performs all ONE TIME cleanup for this test at
> - *		completion or premature exit.
> - ***************************************************************/
>  void cleanup()
>  {
> -	/* Remove them */
> -	char tmppathname[256];
> -	strcpy(tmppathname, pathname);
> -
> -	close(fd);
> -	unlink(testfile);
> -	unlink(testfile2);
> -	unlink(testfile3);
> +	SAFE_CLOSE(NULL, fd);
> +
> +	SAFE_UNLINK(NULL, testfile);
> +
> +	SAFE_UNLINK(NULL, testfile2);
> +
> +	SAFE_UNLINK(NULL, testfile3);
> +
>  	rmdir(pathname);

Make use of tst_rmdir() here instead of the unlink and rmdir.

> -	/*
> -	 * print timing stats if that option was specified.
> -	 * print errno log if that option was specified.
> -	 */
>  	TEST_CLEANUP;
> -
>  }

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Sponsored by Intel(R) XDK 
Develop, test and display web and hybrid apps with a single code base.
Download it for free now!
http://pubads.g.doubleclick.net/gampad/clk?id=111408631&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] fchownat/fchownat01.c: cleanup

[chrubis]

Hi!
> > > -uid_t uid;
> > > -gid_t gid;
> > > -
> > > -int myfchownat(int dirfd, const char *filename, uid_t owner, gid_t group,
> > > -	       int flags)
> > > -{
> > > -	return ltp_syscall(__NR_fchownat, dirfd, filename, owner, group, flags);
> > > -}
> > 
> > Hmm, glibc 2.4 (first one with fchownat() support) has been released in
> > 2006. I wonder if seven years is long enough and if we can get rid of
> > the wrapper now... Moreover I'm not sure what is the status of
> > alternative libc implementations.
> > 
> > There still may be enterprise distributions that still have older glibc
> > than that. Anybody out there?
> 
> To my knowledge the oldest RHEL LTP is run against is RHEL5.3,
> which has glibc 2.5.

It seems to be glibc 2.4 for SLES10 SP3, which is just barely enough.

So I would just keep the wrapper for a few more years. Or even better we
should create a configure check for the fchownat() syscall and define
the wrapper only when it's not available.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Sponsored by Intel(R) XDK 
Develop, test and display web and hybrid apps with a single code base.
Download it for free now!
http://pubads.g.doubleclick.net/gampad/clk?id=111408631&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH v2 1/2] fchownat/fchownat01.c: cleanup

[chrubis]

Hi!
>  #define _GNU_SOURCE
>  
> @@ -53,38 +33,40 @@
>  #include <errno.h>
>  #include <string.h>
>  #include <signal.h>
> +
>  #include "test.h"
>  #include "usctest.h"
> -#include "linux_syscall_numbers.h"
> -
> -#define TEST_CASES 6
> -#ifndef AT_FDCWD
> -#define AT_FDCWD -100
> -#endif

You should include the "lapi/fcntl.h" instead of the defines.

> -void setup();
> -void cleanup();
> -void setup_every_copy();
> +#include "safe_macros.h"
> +#include "fchownat.h"
> +
> +#define TESTFILE	"testfile"
> +#define TESTFILE2	"testfile2"
> +
> +static void setup(void);
> +static void cleanup(void);
> +
> +static int dirfd;
> +static int fd;
> +static int no_fd = -1;
> +static int cu_fd = AT_FDCWD;
> +
> +static struct test_case_t {
> +	int ret;
> +	int flag;
> +	int *fds;
> +	char *filenames;
> +} test_cases[] = {
> +	{0, 0, &dirfd, TESTFILE},
> +	{0, 0, &dirfd, TESTFILE2},

These test are identical, aren't they?

> +	{ENOTDIR, 0, &fd, TESTFILE},
> +	{EBADF, 0, &no_fd, TESTFILE},
> +	{EINVAL, 9999, &dirfd, TESTFILE},
> +	{0, 0, &cu_fd, TESTFILE},
> +};
>  
>  char *TCID = "fchownat01";
> -int TST_TOTAL = TEST_CASES;
> -char pathname[256];
> -char testfile[256];
> -char testfile2[256];
> -char testfile3[256];
> -int dirfd, fd, ret;
> -int fds[TEST_CASES];
> -char *filenames[TEST_CASES];
> -int expected_errno[TEST_CASES] = { 0, 0, ENOTDIR, EBADF, EINVAL, 0 };
> -int flags[TEST_CASES] = { 0, 0, 0, 0, 9999, 0 };
> -
> -uid_t uid;
> -gid_t gid;
> -
> -int myfchownat(int dirfd, const char *filename, uid_t owner, gid_t group,
> -	       int flags)
> -{
> -	return ltp_syscall(__NR_fchownat, dirfd, filename, owner, group, flags);
> -}
> +int TST_TOTAL = ARRAY_SIZE(test_cases);
> +static void fchownat_verify(const struct test_case_t *);
>  
>  int main(int ac, char **av)
>  {
> @@ -92,153 +74,67 @@ int main(int ac, char **av)
>  	char *msg;
>  	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);
> -	}
> -
> -	/***************************************************************
> -	 * parse standard options
> -	 ***************************************************************/
>  	if ((msg = parse_opts(ac, av, NULL, NULL)) != NULL)
>  		tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);
>  
> -	/***************************************************************
> -	 * perform global setup for test
> -	 ***************************************************************/
>  	setup();
>  
> -	/***************************************************************
> -	 * check looping state if -c option given
> -	 ***************************************************************/
>  	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -		setup_every_copy();
> -
>  		tst_count = 0;
> -
> -		/*
> -		 * Call fchownat
> -		 */
> -		for (i = 0; i < TST_TOTAL; i++) {
> -			TEST(myfchownat
> -			     (fds[i], filenames[i], uid, gid, flags[i]));
> -
> -			/* check return code */
> -			if (TEST_ERRNO == expected_errno[i]) {
> -
> -				/***************************************************************
> -				 * only perform functional verification if flag set (-f not given)
> -				 ***************************************************************/
> -				if (STD_FUNCTIONAL_TEST) {
> -					/* No Verification test, yet... */
> -					tst_resm(TPASS,
> -						 "fchownat() returned the expected  errno %d: %s",
> -						 TEST_ERRNO,
> -						 strerror(TEST_ERRNO));
> -				}
> -			} else {
> -				TEST_ERROR_LOG(TEST_ERRNO);
> -				tst_resm(TFAIL,
> -					 "fchownat() Failed, errno=%d : %s",
> -					 TEST_ERRNO, strerror(TEST_ERRNO));
> -			}
> -		}
> -
> +		for (i = 0; i < TST_TOTAL; i++)
> +			fchownat_verify(&test_cases[i]);
>  	}
>  
> -	/***************************************************************
> -	 * cleanup and exit
> -	 ***************************************************************/
>  	cleanup();
> -
> -	return (0);
> +	tst_exit();
>  }
>  
> -void setup_every_copy()
> +static void setup(void)
>  {
> -	/* Initialize test dir and file names */
> -	sprintf(pathname, "fchownattestdir%d", getpid());
> -	sprintf(testfile, "fchownattestfile%d.txt", getpid());
> -	sprintf(testfile2, "/tmp/fchownattestfile%d.txt", getpid());
> -	sprintf(testfile3, "fchownattestdir%d/fchownattestfile%d.txt", getpid(),
> -		getpid());
> -
> -	ret = mkdir(pathname, 0700);
> -	if (ret < 0) {
> -		perror("mkdir: ");
> -		exit(-1);
> -	}
> +	if ((tst_kvercmp(2, 6, 16)) < 0)
> +		tst_brkm(TCONF, NULL, "This test needs kernel 2.6.16 or newer");
>  
> -	dirfd = open(pathname, O_DIRECTORY);
> -	if (dirfd < 0) {
> -		perror("open: ");
> -		exit(-1);
> -	}
> +	tst_sig(NOFORK, DEF_HANDLER, cleanup);
>  
> -	fd = open(testfile, O_CREAT | O_RDWR, 0600);
> -	if (fd < 0) {
> -		perror("open: ");
> -		exit(-1);
> -	}
> +	TEST_PAUSE;
>  
> -	fd = open(testfile2, O_CREAT | O_RDWR, 0600);
> -	if (fd < 0) {
> -		perror("open: ");
> -		exit(-1);
> -	}
> +	tst_tmpdir();
>  
> -	fd = open(testfile3, O_CREAT | O_RDWR, 0600);
> -	if (fd < 0) {
> -		perror("open: ");
> -		exit(-1);
> -	}
> +	dirfd = SAFE_OPEN(cleanup, "./", O_DIRECTORY);
>  
> -	fds[0] = fds[1] = fds[4] = dirfd;
> -	fds[2] = fd;
> -	fds[3] = 100;
> -	fds[5] = AT_FDCWD;
> +	SAFE_TOUCH(cleanup, TESTFILE, 0600, NULL);
>  
> -	filenames[0] = filenames[2] = filenames[3] = filenames[4] =
> -	    filenames[5] = testfile;
> -	filenames[1] = testfile2;
> +	SAFE_TOUCH(cleanup, TESTFILE2, 0600, NULL);
> +
> +	fd = SAFE_OPEN(cleanup, "testfile3", O_CREAT | O_RDWR, 0600);
>  }
>  
> -/***************************************************************
> - * setup() - performs all ONE TIME setup for this test.
> - ***************************************************************/
> -void setup()
> +static void fchownat_verify(const struct test_case_t *test)
>  {
> -	/* Set uid and gid */
> -	uid = geteuid();
> -	gid = getegid();
> -
> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
> -
> -	TEST_PAUSE;
> +	TEST(fchownat(*(test->fds), test->filenames, geteuid(),
> +		      getegid(), test->flag));

You are not checking the TEST_RETURN here as well. Given that there are
both possitive and negative testcases it should be 0 if expected errno
is 0 and -1 if expected errno is non-zero.


> +	if (TEST_ERRNO == test->ret) {

Can you also please rename the test->ret to test->exp_errno?

> +		tst_resm(TPASS | TTERRNO,
> +			 "fchownat() returned the expected errno %d: %s",
> +			 TEST_ERRNO, strerror(TEST_ERRNO));
> +	} else {
> +		TEST_ERROR_LOG(TEST_ERRNO);
> +		tst_resm(TFAIL | TTERRNO,
> +			 "fchownat() Failed, errno=%d : %s",
> +			 TEST_ERRNO, strerror(TEST_ERRNO));
> +	}
>  }
>  
> -/***************************************************************
> - * cleanup() - performs all ONE TIME cleanup for this test at
> - *		completion or premature exit.
> - ***************************************************************/
> -void cleanup()
> +static void cleanup(void)
>  {
> -	/* Remove them */
> -	char tmppathname[256];
> -	strcpy(tmppathname, pathname);
> -
> -	close(fd);
> -	unlink(testfile);
> -	unlink(testfile2);
> -	unlink(testfile3);
> -	rmdir(pathname);
> -
> -	/*
> -	 * print timing stats if that option was specified.
> -	 * print errno log if that option was specified.
> -	 */
> -	TEST_CLEANUP;
> +	if (fd > 0)
> +		close(fd);
> +
> +	if (dirfd > 0)
> +		close(dirfd);
>  
> +	tst_rmdir();
> +
> +	TEST_CLEANUP;
>  }
> -- 
> 1.8.4.2
> 
> 
> 

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Flow-based real-time traffic analytics software. Cisco certified tool.
Monitor traffic, SLAs, QoS, Medianet, WAAS etc. with NetFlow Analyzer
Customize your own dashboards, set traffic alerts and generate reports.
Network behavioral analysis & security monitoring. All-in-one tool.
http://pubads.g.doubleclick.net/gampad/clk?id=126839071&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH v2 2/2] fchownat/fchownat02.c: add a new test

[chrubis]

Hi!
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <error.h>
> +#include <stdlib.h>
> +#include <errno.h>
> +#include <string.h>
> +#include <signal.h>
> +#include "test.h"
> +#include "usctest.h"
> +#include "safe_macros.h"
> +#include "fchownat.h"

I've added AT_SYMLINK_NOFOLLOW to lapi/fcntl.h, please include it here.

> +#define TESTFILE	"testfile"
> +#define TESTFILE_LINK	"testfile_link"
> +
> +char *TCID = "fchownat02";
> +int TST_TOTAL = 1;
> +
> +static int dirfd;
> +static uid_t set_uid = 1000;
> +static gid_t set_gid = 1000;
> +static void setup(void);
> +static void cleanup(void);
> +static void test_verify(void);
> +static void fchownat_verify(void);
> +
> +int main(int ac, char **av)
> +{
> +	int lc;
> +	char *msg;
> +	int i;
> +
> +	msg = parse_opts(ac, av, NULL, NULL);
> +	if (msg != NULL)
> +		tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);
> +
> +	setup();
> +
> +	for (lc = 0; TEST_LOOPING(lc); lc++) {
> +		tst_count = 0;
> +		for (i = 0; i < TST_TOTAL; i++)
> +			fchownat_verify();
> +	}
> +
> +	cleanup();
> +	tst_exit();
> +}
> +
> +static void setup(void)
> +{
> +	struct stat c_buf, l_buf;
> +
> +	if ((tst_kvercmp(2, 6, 16)) < 0)
> +		tst_brkm(TCONF, NULL, "This test needs kernel 2.6.16 or newer");
> +
> +	tst_require_root(NULL);
> +
> +	tst_sig(NOFORK, DEF_HANDLER, cleanup);
> +
> +	TEST_PAUSE;
> +
> +	tst_tmpdir();
> +
> +	dirfd = SAFE_OPEN(cleanup, "./", O_DIRECTORY);
> +
> +	SAFE_TOUCH(cleanup, TESTFILE, 0600, NULL);
> +
> +	SAFE_SYMLINK(cleanup, TESTFILE, TESTFILE_LINK);
> +
> +	if (stat(TESTFILE_LINK, &c_buf) < 0)
> +		tst_brkm(TBROK | TERRNO, cleanup, "stat() failed unexpected");
> +	if (lstat(TESTFILE_LINK, &l_buf) < 0)
> +		tst_brkm(TBROK | TERRNO, cleanup, "lstat() failed unexpected");

I've added SAFE_STAT() and SAFE_LSTAT() to safe_macros.h please use them
here.

> +	if (l_buf.st_uid == set_uid || l_buf.st_gid == set_gid) {
> +		tst_brkm(TBROK | TERRNO, cleanup,
> +			 "link_uid(%d) == set_uid(%d) or link_gid(%d) == "
> +			 "set_gid(%d)", l_buf.st_uid, set_uid, l_buf.st_gid,
> +			 set_gid);
> +	}
> +}
> +
> +static void fchownat_verify(void)
> +{
> +	TEST(fchownat(dirfd, TESTFILE_LINK, set_uid, set_gid,
> +		      AT_SYMLINK_NOFOLLOW));
> +
> +	if (TEST_ERRNO != 0) {
> +		tst_resm(TFAIL | TTERRNO, "fchownat() Failed, errno=%d : %s",
> +			 TEST_ERRNO, strerror(TEST_ERRNO));
> +	} else {
> +		test_verify();
> +	}
> +}
> +
> +static void test_verify(void)
> +{
> +	struct stat c_buf, l_buf;
> +
> +	stat(TESTFILE_LINK, &c_buf);
> +	lstat(TESTFILE_LINK, &l_buf);

Safe macros here as well.

> +	if (c_buf.st_uid != set_uid && l_buf.st_uid == set_uid &&
> +	    c_buf.st_gid != set_gid && l_buf.st_gid == set_gid) {
> +		tst_resm(TPASS, "fchownat() test AT_SYMLINK_NOFOLLOW success");
> +	} else {
> +		tst_resm(TFAIL,
> +			 "fchownat() fail with uid: %d link_uid: %d set_uid: "
> +			 "%d | gid: %d link_gid: %d set_gid: %d", c_buf.st_uid,
> +			 l_buf.st_uid, set_uid, c_buf.st_gid, l_buf.st_gid,
> +			 set_gid);
> +	}
> +}
> +
> +static void cleanup(void)
> +{
> +	tst_rmdir();
> +
> +	TEST_CLEANUP;
> +}

Otherwise it's fine, but needs to be applied over the first patch.
(If that wasn't the case I would have fixed the minor issues and pushed
 it allready)

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Flow-based real-time traffic analytics software. Cisco certified tool.
Monitor traffic, SLAs, QoS, Medianet, WAAS etc. with NetFlow Analyzer
Customize your own dashboards, set traffic alerts and generate reports.
Network behavioral analysis & security monitoring. All-in-one tool.
http://pubads.g.doubleclick.net/gampad/clk?id=126839071&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH v3 1/3] include/safe_macros.h: fix mistakes

[chrubis]

Hi!
> * safe_fstat --> safe_stat
> * safe_fstat --> safe_lstat

Thanks for the fix :)

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Flow-based real-time traffic analytics software. Cisco certified tool.
Monitor traffic, SLAs, QoS, Medianet, WAAS etc. with NetFlow Analyzer
Customize your own dashboards, set traffic alerts and generate reports.
Network behavioral analysis & security monitoring. All-in-one tool.
http://pubads.g.doubleclick.net/gampad/clk?id=126839071&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH v3 3/3] fchownat/fchownat02.c: add a new test

[chrubis]

Hi!
> Add AT_SYMLINK_NOFOLLOW test for fchownat(2)
> 
> Signed-off-by: Zeng Linggang <zenglg.jy@cn.fujitsu.com>

Both pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Flow-based real-time traffic analytics software. Cisco certified tool.
Monitor traffic, SLAs, QoS, Medianet, WAAS etc. with NetFlow Analyzer
Customize your own dashboards, set traffic alerts and generate reports.
Network behavioral analysis & security monitoring. All-in-one tool.
http://pubads.g.doubleclick.net/gampad/clk?id=126839071&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list