[LTP] [PATCH 0/2] Increase fsync() coverage - GH#452

[LTP] [PATCH 0/2] Increase fsync() coverage - GH#452

[Martin Doucha]

This patchset partially solves GH issue #452. New test cases: Call fsync()
on open socket, fifo, and closed non-negative file descriptor.

What is not tested (yet): fsync() calls which produce EINTR, EIO, ENOSPC,
EROFS or EDQUOT errors. These errors can only happen while data in kernel
buffers is being written to disk. Producing these errors reliably would
require some control over the block device underlying the file system.

Martin Doucha (2):
  Update syscalls/fsync03 to new API
  Improve coverage in syscalls/fsync03

 testcases/kernel/syscalls/fsync/fsync03.c | 189 +++++++++-------------
 1 file changed, 76 insertions(+), 113 deletions(-)

-- 
2.22.1

[LTP] [PATCH 1/2] Update syscalls/fsync03 to new API

[Martin Doucha]

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

diff --git a/testcases/kernel/syscalls/fsync/fsync03.c b/testcases/kernel/syscalls/fsync/fsync03.c
index 60d15f429..82fd52070 100644
--- a/testcases/kernel/syscalls/fsync/fsync03.c
+++ b/testcases/kernel/syscalls/fsync/fsync03.c
@@ -1,141 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- *
- *   Copyright (c) International Business Machines  Corp., 2001
- *
- *   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
+ *   Copyright (c) Wayne Boyer, International Business Machines  Corp., 2001
+ *   Copyright (c) 2019 Martin Doucha <mdoucha@suse.cz>
  */
 
 /*
- * NAME
- *	fsync03.c
- *
- * DESCRIPTION
- *	Testcase to check that fsync(2) sets errno correctly.
- *
- * ALGORITHM
- *	1. Call fsync() with an invalid fd, and test for EBADF.
- *	2. Call fsync() on a pipe(fd), and expect EINVAL.
- *
- * USAGE:  <for command-line>
- *  fsync03 [-c n] [-e] [-i n] [-I x] [-P x] [-t]
- *     where,  -c n : Run n copies concurrently.
- *             -e   : Turn on errno logging.
- *             -i n : Execute test n times.
- *             -I x : Execute test for x seconds.
- *             -P x : Pause for x seconds between iterations.
- *             -t   : Turn on syscall timing.
- *
- * HISTORY
- *	07/2001 Ported by Wayne Boyer
- *
- * RESTRICTIONS
- *	NONE
+ * 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.
  */
 
 #include <unistd.h>
 #include <errno.h>
-#include "test.h"
-#include "safe_macros.h"
+#include "tst_test.h"
 
-void setup(void);
-void cleanup(void);
+static int pfd[2];		/* fd's for the pipe() call in setup()  */
+static int bfd = -1;		/* an invalid fd                        */
 
-int fd[2];			/* fd's for the pipe() call in setup()  */
-int pfd;			/* holds the value for fd[1]            */
-int bfd = -1;			/* an invalid fd                        */
-
-struct test_case_t {
+struct test_case {
 	int *fd;
 	int error;
 } TC[] = {
 	/* EBADF - fd is invalid (-1) */
-	{
-	&bfd, EBADF},
-	    /* EINVAL - fsync() on pipe should not succeed. */
-	{
-	&pfd, EINVAL}
+	{&bfd, EBADF},
+	/* EINVAL - fsync() on pipe should not succeed. */
+	{pfd, EINVAL}
 };
 
-char *TCID = "fsync03";
-int TST_TOTAL = 2;
-
-int main(int ac, char **av)
-{
-	int lc;
-	int i;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-
-		tst_count = 0;
+static void test_fsync(unsigned int n) {
+	struct test_case *tc = TC + n;
 
-		/* loop through the test cases */
-		for (i = 0; i < TST_TOTAL; i++) {
-
-			TEST(fsync(*(TC[i].fd)));
-
-			if (TEST_RETURN != -1) {
-				tst_resm(TFAIL, "call succeeded unexpectedly");
-				continue;
-			}
-
-			if (TEST_ERRNO == TC[i].error) {
-				tst_resm(TPASS, "expected failure - "
-					 "errno = %d : %s", TEST_ERRNO,
-					 strerror(TEST_ERRNO));
-			} else {
-				tst_resm(TFAIL, "unexpected error - %d : %s - "
-					 "expected %d", TEST_ERRNO,
-					 strerror(TEST_ERRNO), TC[i].error);
-			}
-		}
+	if (!fsync(*tc->fd)) {
+		tst_res(TFAIL, "fsync() succeeded unexpectedly");
+	} else if (errno != tc->error) {
+		tst_res(TFAIL | TERRNO, "Unexpected error");
+	} else {
+		tst_res(TPASS, "fsync() failed as expected");
 	}
-	cleanup();
-
-	tst_exit();
 }
 
-/*
- * setup() - performs all ONE TIME setup for this test.
- */
-void setup(void)
-{
-
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
-
-	TEST_PAUSE;
-
-	/* make a temporary directory and cd to it */
-	tst_tmpdir();
-
-	SAFE_PIPE(cleanup, fd);
-
-	pfd = fd[1];
+static void setup(void) {
+	SAFE_PIPE(pfd);
 }
 
-/*
- * cleanup() - performs all ONE TIME cleanup for this test at
- *	       completion or premature exit.
- */
-void cleanup(void)
-{
-
-	/* delete the test directory created in setup() */
-	tst_rmdir();
-
+static void cleanup(void) {
+	close(pfd[0]);
+	close(pfd[1]);
 }
+
+static struct tst_test test = {
+	.test = test_fsync,
+	.tcnt = ARRAY_SIZE(TC),
+	.setup = setup,
+	.cleanup = cleanup
+};
-- 
2.22.1

[LTP] [PATCH 2/2] Improve coverage in syscalls/fsync03

[Martin Doucha]

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(-)

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
 };
-- 
2.22.1

[LTP] [PATCH 1/2] Update syscalls/fsync03 to new API

[Cyril Hrubis]

Hi!
> Signed-off-by: Martin Doucha <mdoucha@suse.com>
> ---
>  testcases/kernel/syscalls/fsync/fsync03.c | 152 +++++-----------------
>  1 file changed, 34 insertions(+), 118 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/fsync/fsync03.c b/testcases/kernel/syscalls/fsync/fsync03.c
> index 60d15f429..82fd52070 100644
> --- a/testcases/kernel/syscalls/fsync/fsync03.c
> +++ b/testcases/kernel/syscalls/fsync/fsync03.c
> @@ -1,141 +1,57 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
>  /*
> - *
> - *   Copyright (c) International Business Machines  Corp., 2001
> - *
> - *   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
> + *   Copyright (c) Wayne Boyer, International Business Machines  Corp., 2001
> + *   Copyright (c) 2019 Martin Doucha <mdoucha@suse.cz>
>   */
>  
>  /*
> - * NAME
> - *	fsync03.c
> - *
> - * DESCRIPTION
> - *	Testcase to check that fsync(2) sets errno correctly.
> - *
> - * ALGORITHM
> - *	1. Call fsync() with an invalid fd, and test for EBADF.
> - *	2. Call fsync() on a pipe(fd), and expect EINVAL.
> - *
> - * USAGE:  <for command-line>
> - *  fsync03 [-c n] [-e] [-i n] [-I x] [-P x] [-t]
> - *     where,  -c n : Run n copies concurrently.
> - *             -e   : Turn on errno logging.
> - *             -i n : Execute test n times.
> - *             -I x : Execute test for x seconds.
> - *             -P x : Pause for x seconds between iterations.
> - *             -t   : Turn on syscall timing.
> - *
> - * HISTORY
> - *	07/2001 Ported by Wayne Boyer
> - *
> - * RESTRICTIONS
> - *	NONE
> + * 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.
>   */
>  
>  #include <unistd.h>
>  #include <errno.h>
> -#include "test.h"
> -#include "safe_macros.h"
> +#include "tst_test.h"
>  
> -void setup(void);
> -void cleanup(void);
> +static int pfd[2];		/* fd's for the pipe() call in setup()  */
> +static int bfd = -1;		/* an invalid fd                        */
                                  ^
				  I would say that these two comments
				  are overcommenting.

> -int fd[2];			/* fd's for the pipe() call in setup()  */
> -int pfd;			/* holds the value for fd[1]            */
> -int bfd = -1;			/* an invalid fd                        */
> -
> -struct test_case_t {
> +struct test_case {
>  	int *fd;
>  	int error;
>  } TC[] = {
>  	/* EBADF - fd is invalid (-1) */
> -	{
> -	&bfd, EBADF},
> -	    /* EINVAL - fsync() on pipe should not succeed. */
> -	{
> -	&pfd, EINVAL}
> +	{&bfd, EBADF},
> +	/* EINVAL - fsync() on pipe should not succeed. */
> +	{pfd, EINVAL}
>  };
>  
> -char *TCID = "fsync03";
> -int TST_TOTAL = 2;
> -
> -int main(int ac, char **av)
> -{
> -	int lc;
> -	int i;
> -
> -	tst_parse_opts(ac, av, NULL, NULL);
> -
> -	setup();
> -
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -
> -		tst_count = 0;
> +static void test_fsync(unsigned int n) {
> +	struct test_case *tc = TC + n;

This opening curly brace should be on a separate line for functions. We
do follow the LKML coding style in LTP, see:

https://www.kernel.org/doc/html/v4.10/process/coding-style.html

You can also use checkpatch script for identifying common problems:

https://github.com/torvalds/linux/blob/master/scripts/checkpatch.pl

> -		/* loop through the test cases */
> -		for (i = 0; i < TST_TOTAL; i++) {
> -
> -			TEST(fsync(*(TC[i].fd)));
> -
> -			if (TEST_RETURN != -1) {
> -				tst_resm(TFAIL, "call succeeded unexpectedly");
> -				continue;
> -			}
> -
> -			if (TEST_ERRNO == TC[i].error) {
> -				tst_resm(TPASS, "expected failure - "
> -					 "errno = %d : %s", TEST_ERRNO,
> -					 strerror(TEST_ERRNO));
> -			} else {
> -				tst_resm(TFAIL, "unexpected error - %d : %s - "
> -					 "expected %d", TEST_ERRNO,
> -					 strerror(TEST_ERRNO), TC[i].error);
> -			}
> -		}
> +	if (!fsync(*tc->fd)) {
> +		tst_res(TFAIL, "fsync() succeeded unexpectedly");
> +	} else if (errno != tc->error) {
> +		tst_res(TFAIL | TERRNO, "Unexpected error");
> +	} else {
> +		tst_res(TPASS, "fsync() failed as expected");
>  	}

I would be a bit more pedantic with the return value, i.e. check that
the failure returns exactly -1 as described in the manual page.

> -	cleanup();
> -
> -	tst_exit();
>  }
>  
> -/*
> - * setup() - performs all ONE TIME setup for this test.
> - */
> -void setup(void)
> -{
> -
> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
> -
> -	TEST_PAUSE;
> -
> -	/* make a temporary directory and cd to it */
> -	tst_tmpdir();
> -
> -	SAFE_PIPE(cleanup, fd);
> -
> -	pfd = fd[1];
> +static void setup(void) {
> +	SAFE_PIPE(pfd);
>  }
>  
> -/*
> - * cleanup() - performs all ONE TIME cleanup for this test at
> - *	       completion or premature exit.
> - */
> -void cleanup(void)
> -{
> -
> -	/* delete the test directory created in setup() */
> -	tst_rmdir();
> -
> +static void cleanup(void) {
> +	close(pfd[0]);
> +	close(pfd[1]);
>  }
> +
> +static struct tst_test test = {
> +	.test = test_fsync,
> +	.tcnt = ARRAY_SIZE(TC),
> +	.setup = setup,
> +	.cleanup = cleanup
> +};
> -- 
> 2.22.1
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 2/2] Improve coverage in syscalls/fsync03

[Cyril Hrubis]

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

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3 0/2] Increase fsync() coverage - GH#452

[Martin Doucha]

This patchset partially solves GH issue #452. New test cases: Call fsync()
on open socket, fifo, and closed non-negative file descriptor.

What is not tested (yet): fsync() calls which produce EINTR, EIO, ENOSPC,
EROFS or EDQUOT errors. These errors can only happen while data in kernel
buffers is being written to disk. Producing these errors reliably would
require some control over the block device underlying the file system.

Changes since v2:
- coding style fixes
- cleaner setup() and cleanup()
- use TEST() macro for checking fsync() result
- pedantic check of fsync() return value

Martin Doucha (2):
  Update syscalls/fsync03 to new API
  Improve coverage in syscalls/fsync03

 testcases/kernel/syscalls/fsync/fsync03.c | 178 ++++++++--------------
 1 file changed, 63 insertions(+), 115 deletions(-)

-- 
2.23.0

[LTP] [PATCH v3 1/2] Update syscalls/fsync03 to new API

[Martin Doucha]

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

diff --git a/testcases/kernel/syscalls/fsync/fsync03.c b/testcases/kernel/syscalls/fsync/fsync03.c
index 60d15f429..04d9a6c22 100644
--- a/testcases/kernel/syscalls/fsync/fsync03.c
+++ b/testcases/kernel/syscalls/fsync/fsync03.c
@@ -1,141 +1,63 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- *
- *   Copyright (c) International Business Machines  Corp., 2001
- *
- *   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
+ *   Copyright (c) Wayne Boyer, International Business Machines  Corp., 2001
+ *   Copyright (c) 2019 SUSE LLC <mdoucha@suse.cz>
  */
 
 /*
- * NAME
- *	fsync03.c
- *
- * DESCRIPTION
- *	Testcase to check that fsync(2) sets errno correctly.
- *
- * ALGORITHM
- *	1. Call fsync() with an invalid fd, and test for EBADF.
- *	2. Call fsync() on a pipe(fd), and expect EINVAL.
- *
- * USAGE:  <for command-line>
- *  fsync03 [-c n] [-e] [-i n] [-I x] [-P x] [-t]
- *     where,  -c n : Run n copies concurrently.
- *             -e   : Turn on errno logging.
- *             -i n : Execute test n times.
- *             -I x : Execute test for x seconds.
- *             -P x : Pause for x seconds between iterations.
- *             -t   : Turn on syscall timing.
- *
- * HISTORY
- *	07/2001 Ported by Wayne Boyer
- *
- * RESTRICTIONS
- *	NONE
+ * 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.
  */
 
 #include <unistd.h>
 #include <errno.h>
-#include "test.h"
-#include "safe_macros.h"
+#include "tst_test.h"
 
-void setup(void);
-void cleanup(void);
+static int pfd[2];
+static int bfd = -1;
 
-int fd[2];			/* fd's for the pipe() call in setup()  */
-int pfd;			/* holds the value for fd[1]            */
-int bfd = -1;			/* an invalid fd                        */
-
-struct test_case_t {
+const struct test_case {
 	int *fd;
 	int error;
 } TC[] = {
 	/* EBADF - fd is invalid (-1) */
-	{
-	&bfd, EBADF},
-	    /* EINVAL - fsync() on pipe should not succeed. */
-	{
-	&pfd, EINVAL}
+	{&bfd, EBADF},
+	/* EINVAL - fsync() on pipe should not succeed. */
+	{pfd, EINVAL}
 };
 
-char *TCID = "fsync03";
-int TST_TOTAL = 2;
-
-int main(int ac, char **av)
+static void test_fsync(unsigned int n)
 {
-	int lc;
-	int i;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-
-		tst_count = 0;
+	const struct test_case *tc = TC + n;
 
-		/* loop through the test cases */
-		for (i = 0; i < TST_TOTAL; i++) {
+	TEST(fsync(*tc->fd));
 
-			TEST(fsync(*(TC[i].fd)));
-
-			if (TEST_RETURN != -1) {
-				tst_resm(TFAIL, "call succeeded unexpectedly");
-				continue;
-			}
-
-			if (TEST_ERRNO == TC[i].error) {
-				tst_resm(TPASS, "expected failure - "
-					 "errno = %d : %s", TEST_ERRNO,
-					 strerror(TEST_ERRNO));
-			} else {
-				tst_resm(TFAIL, "unexpected error - %d : %s - "
-					 "expected %d", TEST_ERRNO,
-					 strerror(TEST_ERRNO), TC[i].error);
-			}
-		}
+	if (TST_RET != -1) {
+		tst_res(TFAIL, "fsync() returned unexpected value %ld",
+			TST_RET);
+	} else if (TST_ERR != tc->error) {
+		tst_res(TFAIL | TTERRNO, "fsync(): unexpected error");
+	} else {
+		tst_res(TPASS, "fsync() failed as expected");
 	}
-	cleanup();
-
-	tst_exit();
 }
 
-/*
- * setup() - performs all ONE TIME setup for this test.
- */
-void setup(void)
+static void setup(void)
 {
-
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
-
-	TEST_PAUSE;
-
-	/* make a temporary directory and cd to it */
-	tst_tmpdir();
-
-	SAFE_PIPE(cleanup, fd);
-
-	pfd = fd[1];
+	SAFE_PIPE(pfd);
 }
 
-/*
- * cleanup() - performs all ONE TIME cleanup for this test at
- *	       completion or premature exit.
- */
-void cleanup(void)
+static void cleanup(void)
 {
-
-	/* delete the test directory created in setup() */
-	tst_rmdir();
-
+	close(pfd[0]);
+	close(pfd[1]);
 }
+
+static struct tst_test test = {
+	.test = test_fsync,
+	.tcnt = ARRAY_SIZE(TC),
+	.setup = setup,
+	.cleanup = cleanup
+};
-- 
2.23.0

[LTP] [PATCH v3 2/2] Improve coverage in syscalls/fsync03

[Martin Doucha]

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 | 62 ++++++++++++++++-------
 1 file changed, 44 insertions(+), 18 deletions(-)

diff --git a/testcases/kernel/syscalls/fsync/fsync03.c b/testcases/kernel/syscalls/fsync/fsync03.c
index 04d9a6c22..5636726ce 100644
--- a/testcases/kernel/syscalls/fsync/fsync03.c
+++ b/testcases/kernel/syscalls/fsync/fsync03.c
@@ -7,30 +7,58 @@
 /*
  * 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];
-static int bfd = -1;
+#define FIFO_PATH "fifo"
 
-const struct test_case {
+static int fifo_rfd, fifo_wfd;
+static int pipe_fd[2];
+static int sock_fd, bad_fd = -1;
+
+static const struct test_case {
 	int *fd;
 	int error;
-} TC[] = {
-	/* EBADF - fd is invalid (-1) */
-	{&bfd, EBADF},
+} testcase_list[] = {
 	/* EINVAL - fsync() on pipe should not succeed. */
-	{pfd, EINVAL}
+	{&pipe_fd[1], EINVAL},
+	/* EINVAL - fsync() on socket should not succeed. */
+	{&sock_fd, EINVAL},
+	/* EBADF - fd is closed */
+	{&pipe_fd[0], EBADF},
+	/* EBADF - fd is invalid (-1) */
+	{&bad_fd, EBADF},
+	/* EINVAL - fsync() on fifo should not succeed. */
+	{&fifo_wfd, EINVAL},
 };
 
+static void setup(void)
+{
+	SAFE_MKFIFO(FIFO_PATH, 0644);
+	SAFE_PIPE(pipe_fd);
+
+	// FIFO must be opened for reading first, otherwise
+	// open(fifo, O_WRONLY) will block.
+	fifo_rfd = SAFE_OPEN(FIFO_PATH, O_RDONLY | O_NONBLOCK);
+	fifo_wfd = SAFE_OPEN(FIFO_PATH, O_WRONLY);
+	sock_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(pipe_fd[0]);
+}
+
 static void test_fsync(unsigned int n)
 {
-	const struct test_case *tc = TC + n;
+	const struct test_case *tc = testcase_list + n;
 
 	TEST(fsync(*tc->fd));
 
@@ -44,20 +72,18 @@ static void test_fsync(unsigned int n)
 	}
 }
 
-static void setup(void)
-{
-	SAFE_PIPE(pfd);
-}
-
 static void cleanup(void)
 {
-	close(pfd[0]);
-	close(pfd[1]);
+	SAFE_CLOSE(fifo_wfd);
+	SAFE_CLOSE(fifo_rfd);
+	SAFE_CLOSE(pipe_fd[1]);
+	SAFE_CLOSE(sock_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.23.0

[LTP] [PATCH v3 2/2] Improve coverage in syscalls/fsync03

[Cyril Hrubis]

Hi!
Both pushed, thanks.

I've did a minor change to the code so that we print TTERRNO in case of
the success to make the messages a bit more useful.

-- 
Cyril Hrubis
chrubis@suse.cz