diff mbox series

[2/2] Improve coverage in syscalls/fsync03

Message ID 20190917101706.10013-3-mdoucha@suse.cz
State Superseded
Delegated to: Petr Vorel
Headers show
Series Increase fsync() coverage - GH#452 | expand

Commit Message

Martin Doucha Sept. 17, 2019, 10:17 a.m. UTC
Adds the following test cases where fsync is supposed to fail:
- Closed non-negative file descriptor (EBADF)
- FIFO (EINVAL)
- Socket (EINVAL)

Signed-off-by: Martin Doucha <mdoucha@suse.com>
---
 testcases/kernel/syscalls/fsync/fsync03.c | 83 ++++++++++++++++++-----
 1 file changed, 65 insertions(+), 18 deletions(-)

Comments

Cyril Hrubis Oct. 9, 2019, 3:24 p.m. UTC | #1
Hi!
>  #include <unistd.h>
>  #include <errno.h>
>  #include "tst_test.h"
>  
> -static int pfd[2];		/* fd's for the pipe() call in setup()  */
> -static int bfd = -1;		/* an invalid fd                        */
> +#define FIFO_PATH "fifo"
> +
> +#define PIPE_CASE 0
> +#define SOCKET_CASE 1
> +#define CLOSED_CASE 2
> +
> +/* fd's for the pipe() call in setup()  */
> +static int pfd[2];
> +/* FIFO must be opened for reading first, otherwise open(fifo, O_WRONLY)
> +   will block. */
> +static int fifo_rfd;
>  
>  struct test_case {
> -	int *fd;
> +	int fd;
>  	int error;
> -} TC[] = {
> -	/* EBADF - fd is invalid (-1) */
> -	{&bfd, EBADF},
> +	const char *path;
> +} testcase_list[] = {
>  	/* EINVAL - fsync() on pipe should not succeed. */
> -	{pfd, EINVAL}
> +	{-1, EINVAL, NULL},
> +	/* EINVAL - fsync() on socket should not succeed. */
> +	{-1, EINVAL, NULL},
> +	/* EBADF - fd is closed */
> +	{-1, EBADF, NULL},
> +	/* EBADF - fd is invalid (-1) */
> +	{-1, EBADF, NULL},
> +	/* EINVAL - fsync() on fifo should not succeed. */
> +	{-1, EINVAL, FIFO_PATH},
>  };

Why the change from fd pointer to fd here? AFAIC this is not cleaner nor
shorter solution.

> +static void setup(void) {
> +	SAFE_MKFIFO(FIFO_PATH, 0644);
> +	SAFE_PIPE(pfd);
> +
> +	testcase_list[CLOSED_CASE].fd = pfd[0];
> +	testcase_list[PIPE_CASE].fd = pfd[1];
> +	fifo_rfd = SAFE_OPEN(FIFO_PATH, O_RDONLY | O_NONBLOCK);
> +	testcase_list[SOCKET_CASE].fd = SAFE_SOCKET(AF_UNIX, SOCK_STREAM, 0);
> +
> +	// Do not open any file descriptors after this line unless you close
> +	// them before the next test run.
> +	SAFE_CLOSE(testcase_list[CLOSED_CASE].fd);
> +}
> +
>  static void test_fsync(unsigned int n) {
> -	struct test_case *tc = TC + n;
> +	struct test_case *tc = testcase_list + n;
> +	int fd = tc->fd, result;
>  
> -	if (!fsync(*tc->fd)) {
> +	if (tc->path) {
> +		fd = SAFE_OPEN(tc->path, O_WRONLY);
> +	}
> +
> +	result = fsync(fd);
> +
> +	if (tc->path) {
> +		close(fd);
> +	}
> +
> +	if (!result) {
>  		tst_res(TFAIL, "fsync() succeeded unexpectedly");
>  	} else if (errno != tc->error) {
>  		tst_res(TFAIL | TERRNO, "Unexpected error");
> @@ -40,18 +84,21 @@ static void test_fsync(unsigned int n) {
>  	}
>  }
>  
> -static void setup(void) {
> -	SAFE_PIPE(pfd);
> -}
> -
>  static void cleanup(void) {
> -	close(pfd[0]);
> -	close(pfd[1]);
> +	// close fifo_rfd instead of the already closed test FD
> +	testcase_list[CLOSED_CASE].fd = fifo_rfd;

And I do not like this trickery a much.

> +	for (int i = 0; i < ARRAY_SIZE(testcase_list); i++) {
> +		if (testcase_list[i].fd >= 0) {
> +			close(testcase_list[i].fd);
> +		}
> +	}
>  }
>  
>  static struct tst_test test = {
>  	.test = test_fsync,
> -	.tcnt = ARRAY_SIZE(TC),
> +	.tcnt = ARRAY_SIZE(testcase_list),
> +	.needs_tmpdir = 1,
>  	.setup = setup,
>  	.cleanup = cleanup
>  };
> -- 
> 2.22.1
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/fsync/fsync03.c b/testcases/kernel/syscalls/fsync/fsync03.c
index 82fd52070..d574b07d7 100644
--- a/testcases/kernel/syscalls/fsync/fsync03.c
+++ b/testcases/kernel/syscalls/fsync/fsync03.c
@@ -7,31 +7,75 @@ 
 /*
  * Test Description:
  *  Testcase to check that fsync(2) sets errno correctly.
- *  1. Call fsync() with an invalid fd, and test for EBADF.
- *  2. Call fsync() on a pipe(fd), and expect EINVAL.
+ *  1. Call fsync() on a pipe(fd), and expect EINVAL.
+ *  2. Call fsync() on a socket(fd), and expect EINVAL.
+ *  3. Call fsync() on a closed fd, and test for EBADF.
+ *  4. Call fsync() on an invalid fd, and test for EBADF.
+ *  5. Call fsync() on a fifo(fd), and expect EINVAL.
  */
 
 #include <unistd.h>
 #include <errno.h>
 #include "tst_test.h"
 
-static int pfd[2];		/* fd's for the pipe() call in setup()  */
-static int bfd = -1;		/* an invalid fd                        */
+#define FIFO_PATH "fifo"
+
+#define PIPE_CASE 0
+#define SOCKET_CASE 1
+#define CLOSED_CASE 2
+
+/* fd's for the pipe() call in setup()  */
+static int pfd[2];
+/* FIFO must be opened for reading first, otherwise open(fifo, O_WRONLY)
+   will block. */
+static int fifo_rfd;
 
 struct test_case {
-	int *fd;
+	int fd;
 	int error;
-} TC[] = {
-	/* EBADF - fd is invalid (-1) */
-	{&bfd, EBADF},
+	const char *path;
+} testcase_list[] = {
 	/* EINVAL - fsync() on pipe should not succeed. */
-	{pfd, EINVAL}
+	{-1, EINVAL, NULL},
+	/* EINVAL - fsync() on socket should not succeed. */
+	{-1, EINVAL, NULL},
+	/* EBADF - fd is closed */
+	{-1, EBADF, NULL},
+	/* EBADF - fd is invalid (-1) */
+	{-1, EBADF, NULL},
+	/* EINVAL - fsync() on fifo should not succeed. */
+	{-1, EINVAL, FIFO_PATH},
 };
 
+static void setup(void) {
+	SAFE_MKFIFO(FIFO_PATH, 0644);
+	SAFE_PIPE(pfd);
+
+	testcase_list[CLOSED_CASE].fd = pfd[0];
+	testcase_list[PIPE_CASE].fd = pfd[1];
+	fifo_rfd = SAFE_OPEN(FIFO_PATH, O_RDONLY | O_NONBLOCK);
+	testcase_list[SOCKET_CASE].fd = SAFE_SOCKET(AF_UNIX, SOCK_STREAM, 0);
+
+	// Do not open any file descriptors after this line unless you close
+	// them before the next test run.
+	SAFE_CLOSE(testcase_list[CLOSED_CASE].fd);
+}
+
 static void test_fsync(unsigned int n) {
-	struct test_case *tc = TC + n;
+	struct test_case *tc = testcase_list + n;
+	int fd = tc->fd, result;
 
-	if (!fsync(*tc->fd)) {
+	if (tc->path) {
+		fd = SAFE_OPEN(tc->path, O_WRONLY);
+	}
+
+	result = fsync(fd);
+
+	if (tc->path) {
+		close(fd);
+	}
+
+	if (!result) {
 		tst_res(TFAIL, "fsync() succeeded unexpectedly");
 	} else if (errno != tc->error) {
 		tst_res(TFAIL | TERRNO, "Unexpected error");
@@ -40,18 +84,21 @@  static void test_fsync(unsigned int n) {
 	}
 }
 
-static void setup(void) {
-	SAFE_PIPE(pfd);
-}
-
 static void cleanup(void) {
-	close(pfd[0]);
-	close(pfd[1]);
+	// close fifo_rfd instead of the already closed test FD
+	testcase_list[CLOSED_CASE].fd = fifo_rfd;
+
+	for (int i = 0; i < ARRAY_SIZE(testcase_list); i++) {
+		if (testcase_list[i].fd >= 0) {
+			close(testcase_list[i].fd);
+		}
+	}
 }
 
 static struct tst_test test = {
 	.test = test_fsync,
-	.tcnt = ARRAY_SIZE(TC),
+	.tcnt = ARRAY_SIZE(testcase_list),
+	.needs_tmpdir = 1,
 	.setup = setup,
 	.cleanup = cleanup
 };