Skip to contentSkip to conversation

syscalls/fstat{01,02}: rewrote testcases

PATCH New Christian Amann 2 participants 4 messages Jun 6, 2019

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/2 syscalls/fstat{01,02}: rewrote testcases

Merged the two tests:
fstat01 is a subset of fstat02 and is therefore not needed anymore.
Rewrote testcase:
Cleaned up and ported to new library.

---

Notes:
Removed test for raw fstat syscall:
Calling the raw syscall on a glibc provided stat struct on a 32 bit system caused either an overflow of the structure or the values were wrong due to them having a bigger size than the syscalls anticipated (tested all three: oldfstat/fstat/fstat64). By providing custom stat structs I would have to make sure they work on all architectures (same as glibc does). I decided to remove this test variant because I couldn't guarantee this.

 runtest/syscalls                           |   2 -
 testcases/kernel/syscalls/fstat/.gitignore |   2 -
 testcases/kernel/syscalls/fstat/fstat01.c  | 175 -----------------------------
 testcases/kernel/syscalls/fstat/fstat02.c  | 144 +++++++-----------------
 4 files changed, 43 insertions(+), 280 deletions(-)
 delete mode 100644 testcases/kernel/syscalls/fstat/fstat01.c
runtest/syscalls +0 -2
index 762b15b1f..6d85f9bb7 100644
@@ -332,8 +332,6 @@ fpathconf01 fpathconf01
 fremovexattr01 fremovexattr01
 fremovexattr02 fremovexattr02
 
-fstat01 fstat01
-fstat01_64 fstat01_64
 fstat02 fstat02
 fstat02_64 fstat02_64
 fstat03 fstat03
testcases/kernel/syscalls/fstat/.gitignore +0 -2
index cf5205d44..1c66bdf10 100644
@@ -1,5 +1,3 @@
-/fstat01
-/fstat01_64
 /fstat02
 /fstat02_64
 /fstat03
testcases/kernel/syscalls/fstat/fstat01.c deleted +0 -175

175 lines not on this page · raw message

testcases/kernel/syscalls/fstat/fstat02.c +43 -101

195 lines not on this page · raw message

Signature · 1 line
2.16.4
#1 lore raw

PATCH v2 2/2 syscalls/fstat{03,05}: rewrote testcases

Merged fstat03 and fstat05:
Both testcases had lots of duplicated code which could be removed by creating a single testcase for both scenarios.
Rewrote testcase:
Cleaned up and ported to new library.

---
 runtest/syscalls                           |   2 -
 testcases/kernel/syscalls/fstat/.gitignore |   2 -
 testcases/kernel/syscalls/fstat/fstat03.c  | 207 +++++++++---------------
 testcases/kernel/syscalls/fstat/fstat05.c  | 244 -----------------------------
 4 files changed, 75 insertions(+), 380 deletions(-)
 delete mode 100644 testcases/kernel/syscalls/fstat/fstat05.c
runtest/syscalls +0 -2
index 6d85f9bb7..05600f9dc 100644
@@ -336,8 +336,6 @@ fstat02 fstat02
 fstat02_64 fstat02_64
 fstat03 fstat03
 fstat03_64 fstat03_64
-fstat05 fstat05
-fstat05_64 fstat05_64
 
 #fstatat64/newfstatat test cases
 fstatat01 fstatat01
testcases/kernel/syscalls/fstat/.gitignore +0 -2
index 1c66bdf10..9b1089438 100644
@@ -2,5 +2,3 @@
 /fstat02_64
 /fstat03
 /fstat03_64
-/fstat05
-/fstat05_64
testcases/kernel/syscalls/fstat/fstat03.c +75 -132

235 lines not on this page · raw message

testcases/kernel/syscalls/fstat/fstat05.c deleted +0 -244

244 lines not on this page · raw message

Signature · 1 line
2.16.4
#2 lore raw

PATCH v2 2/2 syscalls/fstat{03,05}: rewrote testcases

Hi!

Quoted 5 lines
testcases/kernel/syscalls/fstat/fstat03.c +1 -3
+static void sighandler(int sig LTP_ATTRIBUTE_UNUSED)
 {
-	struct stat stat_buf;	/* stat structure buffer */
-	int lc;
-

testcases/kernel/syscalls/fstat/fstat03.c line 43

Quoted 10 lines
testcases/kernel/syscalls/fstat/fstat03.c +3 -7
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	/*
-	 * Invoke setup function to create a testfile under temporary
-	 * directory.
-	 */
-	setup();
+	tst_res(TPASS, "fstat() failed as expected with SIGSEGV");
+	exit(0);
+}

Technically you should not use anything but signal-async-safe functions in signal handlers and while this would work fine most of the time it may deadlock the process under certain circumstatnces.

So the best way how to handle this is to let the child segfault then do SAFE_WAITPID() and examine the status value.

Quoted 89 lines
testcases/kernel/syscalls/fstat/fstat03.c +25 -53
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
+static void check_fstat(unsigned int tc_num)
+{
+	struct tcase *tc = &tcases[tc_num];
 
-		tst_count = 0;
-		/*
-		 * Call fstat(2) to get the status information
-		 * of a closed testfile pointed to by 'fd'.
-		 * verify that fstat fails with -1 return value and
-		 * sets appropriate errno.
-		 */
-		TEST(fstat(fildes, &stat_buf));
+	if (tc_num == 1)
+		signal(SIGSEGV, &sighandler);
 
-		/* Check return code from fstat(2) */
-		if (TEST_RETURN == -1) {
-			if (TEST_ERRNO == EBADF) {
-				tst_resm(TPASS,
-					 "fstat() fails with expected error EBADF");
-			} else {
-				tst_resm(TFAIL | TTERRNO,
-					 "fstat() did not fail with EBADF");
-			}
+	TEST(fstat(*tc->fd, tc->stat_buf));
+	if (TST_RET == -1) {
+		if (tc->exp_err == TST_ERR) {
+			tst_res(TPASS,
+				 "fstat() fails with expected error %s",
+				 tst_strerrno(tc->exp_err));
 		} else {
-			tst_resm(TFAIL, "fstat() returned %ld, expected -1",
-				 TEST_RETURN);
+			tst_res(TFAIL | TTERRNO,
+				 "fstat() did not fail with %s, but with",
+				 tst_strerrno(tc->exp_err));
 		}
+	} else {
+		tst_res(TFAIL, "fstat() returned %ld, expected -1",
+			 TST_RET);
 	}
-
-	/*
-	 * Invoke cleanup() to delete the test directory/file(s) created
-	 * in the setup().
-	 */
-	cleanup();
-
-	tst_exit();
 }
 
-/*
- * void
- * setup(void) - performs all ONE TIME setup for this test.
- *	Exit the test program on receipt of unexpected signals.
- *	Create a temporary directory and change directory to it.
- *      Create a testfile under temporary directory.
- *      Close the testfile.
- */
-void setup(void)
+static void run(unsigned int tc_num)
 {
-	/* Capture unexpected signals */
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
-
-	TEST_PAUSE;
-
-	/* Make a temp dir and cd to it */
-	tst_tmpdir();
+	pid_t pid;
 
-	/* Create a testfile under temporary directory */
-	fildes = SAFE_OPEN(cleanup, TEST_FILE, O_RDWR | O_CREAT, 0666);
-
-	SAFE_CLOSE(cleanup, fildes);
+	pid = SAFE_FORK();
+	if (pid == 0) {
+		check_fstat(tc_num);
+		return;
+	}
+	tst_reap_children();
 }
 
-/*
- * void
- * cleanup() - Performs all ONE TIME cleanup for this test at
- *             completion or premature exit.
- *	Print test timing stats and errno log if test executed with options.

testcases/kernel/syscalls/fstat/fstat03.c line 84

Quoted 10 lines
testcases/kernel/syscalls/fstat/fstat03.c +3 -6
- *	Close the testfile if still opened.
- *	Remove temporary directory and sub-directories/files under it
- *	created during setup().
- *	Exit the test program with normal exit code.
- */
-void cleanup(void)
+static void setup(void)
 {
+	fd_ebadf = SAFE_OPEN(TESTFILE, O_RDWR | O_CREAT, 0644);
+	SAFE_CLOSE(fd_ebadf);

Actually this is kind of useless since the SAFE_CLOSE() sets the fd_ebadfd to -1.

And we even have tst_get_unused_pid() function exactly for this.

Quoted 5 lines
testcases/kernel/syscalls/fstat/fstat03.c +2 -2
+	fd_ok = SAFE_OPEN(TESTFILE, O_RDWR | O_CREAT, 0644);
+}
 
-	tst_rmdir();
-
Signature · 2 lines
Cyril Hrubis
[email protected]
#3 lore raw

PATCH v2 1/2 syscalls/fstat{01,02}: rewrote testcases

In reply to Christian Amann

Hi!
Pushed with minor changes, thanks.

The fstat01 was in the quickhit runtest file as well,, removed that.

And as for the test, we don't have to print another TFAIL message on failure and we should be more strict about checking the return value (the test would pass if fstat returned garbage that != -1)

static void run(void)
{
- int fail;
+ int fail = 0;

TEST(fstat(fildes, &stat_buf));

- if (TST_RET == -1) {
+ if (TST_RET != 0) {
tst_res(TFAIL | TTERRNO, "fstat() failed"); return;
}
@@ -61,10 +61,8 @@ static void run(void)
fail++;
}

- if (fail) {
- tst_res(TFAIL, "fstat() reported wrong values.");
+ if (fail)
return;
- }

tst_res(TPASS, "fstat() reported correct values.");
}

Signature · 2 lines
Cyril Hrubis
[email protected]