All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [LTP] [PATCH 2/2] Make use of SAFE_ACCESS
  2022-03-14 17:28 ` [LTP] [PATCH 2/2] Make use of SAFE_ACCESS Dai Shili
@ 2022-03-14  6:43   ` xuyang2018.jy
  2022-03-17 21:00     ` [LTP] [PATCH v2 1/3] Add SAFE_ACCESS macro Dai Shili
  0 siblings, 1 reply; 27+ messages in thread
From: xuyang2018.jy @ 2022-03-14  6:43 UTC (permalink / raw)
  To: daisl.fnst; +Cc: ltp

Hi Dai

Since only umount2_02 uses old ltp api, I think we can convert it into 
ltp new api. Then we can just add this safe_macro to tst_safe_macro.c 
instead of safe_macro.c.

Best Regards
Yang Xu
> Signed-off-by: Dai Shili<daisl.fnst@fujitsu.com>
> ---
>   testcases/kernel/mem/tunable/max_map_count.c    | 4 +---
>   testcases/kernel/syscalls/fanotify/fanotify17.c | 3 +--
>   testcases/kernel/syscalls/getxattr/getxattr05.c | 3 +--
>   testcases/kernel/syscalls/madvise/madvise08.c   | 3 +--
>   testcases/kernel/syscalls/quotactl/quotactl01.c | 6 ++----
>   testcases/kernel/syscalls/quotactl/quotactl06.c | 3 +--
>   testcases/kernel/syscalls/umount2/umount2_02.c  | 6 ++----
>   7 files changed, 9 insertions(+), 19 deletions(-)
>
> diff --git a/testcases/kernel/mem/tunable/max_map_count.c b/testcases/kernel/mem/tunable/max_map_count.c
> index a4c3dbf..caf8972 100644
> --- a/testcases/kernel/mem/tunable/max_map_count.c
> +++ b/testcases/kernel/mem/tunable/max_map_count.c
> @@ -58,9 +58,7 @@ static long old_overcommit = -1;
>
>   static void setup(void)
>   {
> -	if (access(PATH_SYSVM "max_map_count", F_OK) != 0)
> -		tst_brk(TBROK | TERRNO,
> -			 "Can't support to test max_map_count");
> +	SAFE_ACCESS(PATH_SYSVM "max_map_count", F_OK);
>
>   	old_max_map_count = get_sys_tune("max_map_count");
>   	old_overcommit = get_sys_tune("overcommit_memory");
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify17.c b/testcases/kernel/syscalls/fanotify/fanotify17.c
> index 35beb53..7d74b25 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify17.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify17.c
> @@ -159,8 +159,7 @@ static void do_unshare(int map_root)
>   		 * uid_map file should exist since Linux 3.8 because
>   		 * it is available on Linux 3.5
>   		 */
> -		if (access(UID_MAP, F_OK))
> -			tst_brk(TBROK, "file %s didn't exist", UID_MAP);
> +		SAFE_ACCESS(UID_MAP, F_OK);
>
>   		SAFE_FILE_PRINTF(UID_MAP, "%d %d %d", 0, 0, 1);
>   	}
> diff --git a/testcases/kernel/syscalls/getxattr/getxattr05.c b/testcases/kernel/syscalls/getxattr/getxattr05.c
> index c0d339f..3e73cc5 100644
> --- a/testcases/kernel/syscalls/getxattr/getxattr05.c
> +++ b/testcases/kernel/syscalls/getxattr/getxattr05.c
> @@ -100,8 +100,7 @@ static void do_unshare(int map_root)
>   		/* uid_map file should exist since Linux 3.8 because
>   		 * it is available on Linux 3.5
>   		 */
> -		if (access(UID_MAP, F_OK))
> -			tst_brk(TBROK, "file %s didn't exist", UID_MAP);
> +		SAFE_ACCESS(UID_MAP, F_OK);
>
>   		SAFE_FILE_PRINTF(UID_MAP, "%d %d %d", 0, 0, 1);
>   	}
> diff --git a/testcases/kernel/syscalls/madvise/madvise08.c b/testcases/kernel/syscalls/madvise/madvise08.c
> index 30f3604..10549f4 100644
> --- a/testcases/kernel/syscalls/madvise/madvise08.c
> +++ b/testcases/kernel/syscalls/madvise/madvise08.c
> @@ -110,8 +110,7 @@ static int find_sequence(int pid)
>
>   	snprintf(dumpname, 256, "dump-%d", pid);
>   	tst_res(TINFO, "Dump file should be %s", dumpname);
> -	if (access(dumpname, F_OK))
> -		tst_brk(TBROK | TERRNO, "Dump file was not found.");
> +	SAFE_ACCESS(dumpname, F_OK);
>
>   	dfd = SAFE_OPEN(dumpname, O_RDONLY);
>
> diff --git a/testcases/kernel/syscalls/quotactl/quotactl01.c b/testcases/kernel/syscalls/quotactl/quotactl01.c
> index 561e503..3382d7b 100644
> --- a/testcases/kernel/syscalls/quotactl/quotactl01.c
> +++ b/testcases/kernel/syscalls/quotactl/quotactl01.c
> @@ -160,11 +160,9 @@ static void setup(void)
>   	SAFE_CMD(cmd, NULL, NULL);
>   	fmt_id = var->fmt_id;
>
> -	if (access(USRPATH, F_OK) == -1)
> -		tst_brk(TFAIL | TERRNO, "user quotafile didn't exist");
> +	SAFE_ACCESS(USRPATH, F_OK);
>
> -	if (access(GRPPATH, F_OK) == -1)
> -		tst_brk(TFAIL | TERRNO, "group quotafile didn't exist");
> +	SAFE_ACCESS(GRPPATH, F_OK);
>
>   	tst_require_quota_support(tst_device->dev, fmt_id, usrpath);
>
> diff --git a/testcases/kernel/syscalls/quotactl/quotactl06.c b/testcases/kernel/syscalls/quotactl/quotactl06.c
> index 8771523..839c6ed 100644
> --- a/testcases/kernel/syscalls/quotactl/quotactl06.c
> +++ b/testcases/kernel/syscalls/quotactl/quotactl06.c
> @@ -186,8 +186,7 @@ static void setup(void)
>   	/* vfsv0 block limit 2^42, vfsv1 block limit 2^63 - 1 */
>   	set_dqmax.dqb_bsoftlimit = tst_variant ? 0x20000000000000 : 0x100000000;
>
> -	if (access(USRPATH, F_OK) == -1)
> -		tst_brk(TBROK | TERRNO, "user quotafile didn't exist");
> +	SAFE_ACCESS(USRPATH, F_OK);
>
>   	tst_require_quota_support(tst_device->dev, fmt_id, usrpath);
>
> diff --git a/testcases/kernel/syscalls/umount2/umount2_02.c b/testcases/kernel/syscalls/umount2/umount2_02.c
> index 7d558fa..9dad4d5 100644
> --- a/testcases/kernel/syscalls/umount2/umount2_02.c
> +++ b/testcases/kernel/syscalls/umount2/umount2_02.c
> @@ -128,10 +128,8 @@ static void setup(void)
>   static void test_umount2(int i)
>   {
>   	/* a new access removes the expired mark of the mount point */
> -	if (test_cases[i].do_access) {
> -		if (access(MNTPOINT, F_OK) == -1)
> -			tst_brkm(TBROK | TERRNO, cleanup, "access(2) failed");
> -	}
> +	if (test_cases[i].do_access)
> +		SAFE_ACCESS(cleanup, MNTPOINT, F_OK);
>
>   	TEST(umount2_retry(MNTPOINT, test_cases[i].flag));
>

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [LTP] [PATCH 1/2] Add SAFE_ACCESS macro
@ 2022-03-14 17:28 Dai Shili
  2022-03-14 17:28 ` [LTP] [PATCH 2/2] Make use of SAFE_ACCESS Dai Shili
  0 siblings, 1 reply; 27+ messages in thread
From: Dai Shili @ 2022-03-14 17:28 UTC (permalink / raw)
  To: ltp

Signed-off-by: Dai Shili <daisl.fnst@fujitsu.com>
---
 include/old/safe_macros.h |  3 +++
 include/safe_macros_fn.h  |  3 +++
 include/tst_safe_macros.h |  3 +++
 lib/safe_macros.c         | 20 ++++++++++++++++++++
 4 files changed, 29 insertions(+)

diff --git a/include/old/safe_macros.h b/include/old/safe_macros.h
index fb1d7a1..1d5de49 100644
--- a/include/old/safe_macros.h
+++ b/include/old/safe_macros.h
@@ -20,6 +20,9 @@
 #include "old_safe_stdio.h"
 #include "old_safe_net.h"
 
+#define SAFE_ACCESS(cleanup_fn, pathname, mode)	\
+	safe_access(__FILE__, __LINE__, cleanup_fn, (pathname), (mode))
+
 #define SAFE_BASENAME(cleanup_fn, path)	\
 	safe_basename(__FILE__, __LINE__, (cleanup_fn), (path))
 
diff --git a/include/safe_macros_fn.h b/include/safe_macros_fn.h
index 3df9528..2c8eb50 100644
--- a/include/safe_macros_fn.h
+++ b/include/safe_macros_fn.h
@@ -24,6 +24,9 @@
 #include <unistd.h>
 #include <dirent.h>
 
+int safe_access(const char *file, const int lineno,
+		void (*cleanup_fn)(void), const char *pathname, int mode);
+
 char* safe_basename(const char *file, const int lineno,
                     void (*cleanup_fn)(void), char *path);
 
diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
index 9a543e4..d8e56f5 100644
--- a/include/tst_safe_macros.h
+++ b/include/tst_safe_macros.h
@@ -24,6 +24,9 @@
 #include "safe_macros_fn.h"
 #include "tst_cmd.h"
 
+#define SAFE_ACCESS(pathname, mode) \
+	safe_access(__FILE__, __LINE__, NULL, (pathname), (mode))
+
 #define SAFE_BASENAME(path) \
 	safe_basename(__FILE__, __LINE__, NULL, (path))
 
diff --git a/lib/safe_macros.c b/lib/safe_macros.c
index a5b6bc5..006e583 100644
--- a/lib/safe_macros.c
+++ b/lib/safe_macros.c
@@ -24,6 +24,26 @@
 #include "test.h"
 #include "safe_macros.h"
 
+int
+safe_access(const char *file, const int lineno, void (*cleanup_fn) (void),
+	   const char *pathname, int mode)
+{
+	int rval;
+
+	rval = access(pathname, mode);
+
+	if (rval == -1) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"access(%s,%d) failed", pathname, mode);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid access(%s,%d) return value %d", pathname,
+			mode, rval);
+	}
+
+	return rval;
+}
+
 char *safe_basename(const char *file, const int lineno,
 		    void (*cleanup_fn) (void), char *path)
 {
-- 
1.8.3.1


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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [LTP] [PATCH 2/2] Make use of SAFE_ACCESS
  2022-03-14 17:28 [LTP] [PATCH 1/2] Add SAFE_ACCESS macro Dai Shili
@ 2022-03-14 17:28 ` Dai Shili
  2022-03-14  6:43   ` xuyang2018.jy
  0 siblings, 1 reply; 27+ messages in thread
From: Dai Shili @ 2022-03-14 17:28 UTC (permalink / raw)
  To: ltp

Signed-off-by: Dai Shili <daisl.fnst@fujitsu.com>
---
 testcases/kernel/mem/tunable/max_map_count.c    | 4 +---
 testcases/kernel/syscalls/fanotify/fanotify17.c | 3 +--
 testcases/kernel/syscalls/getxattr/getxattr05.c | 3 +--
 testcases/kernel/syscalls/madvise/madvise08.c   | 3 +--
 testcases/kernel/syscalls/quotactl/quotactl01.c | 6 ++----
 testcases/kernel/syscalls/quotactl/quotactl06.c | 3 +--
 testcases/kernel/syscalls/umount2/umount2_02.c  | 6 ++----
 7 files changed, 9 insertions(+), 19 deletions(-)

diff --git a/testcases/kernel/mem/tunable/max_map_count.c b/testcases/kernel/mem/tunable/max_map_count.c
index a4c3dbf..caf8972 100644
--- a/testcases/kernel/mem/tunable/max_map_count.c
+++ b/testcases/kernel/mem/tunable/max_map_count.c
@@ -58,9 +58,7 @@ static long old_overcommit = -1;
 
 static void setup(void)
 {
-	if (access(PATH_SYSVM "max_map_count", F_OK) != 0)
-		tst_brk(TBROK | TERRNO,
-			 "Can't support to test max_map_count");
+	SAFE_ACCESS(PATH_SYSVM "max_map_count", F_OK);
 
 	old_max_map_count = get_sys_tune("max_map_count");
 	old_overcommit = get_sys_tune("overcommit_memory");
diff --git a/testcases/kernel/syscalls/fanotify/fanotify17.c b/testcases/kernel/syscalls/fanotify/fanotify17.c
index 35beb53..7d74b25 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify17.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify17.c
@@ -159,8 +159,7 @@ static void do_unshare(int map_root)
 		 * uid_map file should exist since Linux 3.8 because
 		 * it is available on Linux 3.5
 		 */
-		if (access(UID_MAP, F_OK))
-			tst_brk(TBROK, "file %s didn't exist", UID_MAP);
+		SAFE_ACCESS(UID_MAP, F_OK);
 
 		SAFE_FILE_PRINTF(UID_MAP, "%d %d %d", 0, 0, 1);
 	}
diff --git a/testcases/kernel/syscalls/getxattr/getxattr05.c b/testcases/kernel/syscalls/getxattr/getxattr05.c
index c0d339f..3e73cc5 100644
--- a/testcases/kernel/syscalls/getxattr/getxattr05.c
+++ b/testcases/kernel/syscalls/getxattr/getxattr05.c
@@ -100,8 +100,7 @@ static void do_unshare(int map_root)
 		/* uid_map file should exist since Linux 3.8 because
 		 * it is available on Linux 3.5
 		 */
-		if (access(UID_MAP, F_OK))
-			tst_brk(TBROK, "file %s didn't exist", UID_MAP);
+		SAFE_ACCESS(UID_MAP, F_OK);
 
 		SAFE_FILE_PRINTF(UID_MAP, "%d %d %d", 0, 0, 1);
 	}
diff --git a/testcases/kernel/syscalls/madvise/madvise08.c b/testcases/kernel/syscalls/madvise/madvise08.c
index 30f3604..10549f4 100644
--- a/testcases/kernel/syscalls/madvise/madvise08.c
+++ b/testcases/kernel/syscalls/madvise/madvise08.c
@@ -110,8 +110,7 @@ static int find_sequence(int pid)
 
 	snprintf(dumpname, 256, "dump-%d", pid);
 	tst_res(TINFO, "Dump file should be %s", dumpname);
-	if (access(dumpname, F_OK))
-		tst_brk(TBROK | TERRNO, "Dump file was not found.");
+	SAFE_ACCESS(dumpname, F_OK);
 
 	dfd = SAFE_OPEN(dumpname, O_RDONLY);
 
diff --git a/testcases/kernel/syscalls/quotactl/quotactl01.c b/testcases/kernel/syscalls/quotactl/quotactl01.c
index 561e503..3382d7b 100644
--- a/testcases/kernel/syscalls/quotactl/quotactl01.c
+++ b/testcases/kernel/syscalls/quotactl/quotactl01.c
@@ -160,11 +160,9 @@ static void setup(void)
 	SAFE_CMD(cmd, NULL, NULL);
 	fmt_id = var->fmt_id;
 
-	if (access(USRPATH, F_OK) == -1)
-		tst_brk(TFAIL | TERRNO, "user quotafile didn't exist");
+	SAFE_ACCESS(USRPATH, F_OK);
 
-	if (access(GRPPATH, F_OK) == -1)
-		tst_brk(TFAIL | TERRNO, "group quotafile didn't exist");
+	SAFE_ACCESS(GRPPATH, F_OK);
 
 	tst_require_quota_support(tst_device->dev, fmt_id, usrpath);
 
diff --git a/testcases/kernel/syscalls/quotactl/quotactl06.c b/testcases/kernel/syscalls/quotactl/quotactl06.c
index 8771523..839c6ed 100644
--- a/testcases/kernel/syscalls/quotactl/quotactl06.c
+++ b/testcases/kernel/syscalls/quotactl/quotactl06.c
@@ -186,8 +186,7 @@ static void setup(void)
 	/* vfsv0 block limit 2^42, vfsv1 block limit 2^63 - 1 */
 	set_dqmax.dqb_bsoftlimit = tst_variant ? 0x20000000000000 : 0x100000000;
 
-	if (access(USRPATH, F_OK) == -1)
-		tst_brk(TBROK | TERRNO, "user quotafile didn't exist");
+	SAFE_ACCESS(USRPATH, F_OK);
 
 	tst_require_quota_support(tst_device->dev, fmt_id, usrpath);
 
diff --git a/testcases/kernel/syscalls/umount2/umount2_02.c b/testcases/kernel/syscalls/umount2/umount2_02.c
index 7d558fa..9dad4d5 100644
--- a/testcases/kernel/syscalls/umount2/umount2_02.c
+++ b/testcases/kernel/syscalls/umount2/umount2_02.c
@@ -128,10 +128,8 @@ static void setup(void)
 static void test_umount2(int i)
 {
 	/* a new access removes the expired mark of the mount point */
-	if (test_cases[i].do_access) {
-		if (access(MNTPOINT, F_OK) == -1)
-			tst_brkm(TBROK | TERRNO, cleanup, "access(2) failed");
-	}
+	if (test_cases[i].do_access)
+		SAFE_ACCESS(cleanup, MNTPOINT, F_OK);
 
 	TEST(umount2_retry(MNTPOINT, test_cases[i].flag));
 
-- 
1.8.3.1


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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [LTP] [PATCH v2 1/3] Add SAFE_ACCESS macro
  2022-03-14  6:43   ` xuyang2018.jy
@ 2022-03-17 21:00     ` Dai Shili
  2022-03-17 21:00       ` [LTP] [PATCH v2 2/3] Make use of SAFE_ACCESS Dai Shili
                         ` (2 more replies)
  0 siblings, 3 replies; 27+ messages in thread
From: Dai Shili @ 2022-03-17 21:00 UTC (permalink / raw)
  To: xuyang2018.jy; +Cc: ltp

Signed-off-by: Dai Shili <daisl.fnst@fujitsu.com>
---
 include/tst_safe_macros.h |  5 +++++
 lib/tst_safe_macros.c     | 19 +++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
index 9a543e4..81c4b08 100644
--- a/include/tst_safe_macros.h
+++ b/include/tst_safe_macros.h
@@ -24,6 +24,11 @@
 #include "safe_macros_fn.h"
 #include "tst_cmd.h"
 
+int safe_access(const char *filename, const int lineno, const char *pathname,
+		   int mode);
+#define SAFE_ACCESS(path, mode) \
+	safe_access(__FILE__, __LINE__, (path), (mode))
+
 #define SAFE_BASENAME(path) \
 	safe_basename(__FILE__, __LINE__, NULL, (path))
 
diff --git a/lib/tst_safe_macros.c b/lib/tst_safe_macros.c
index a9f6aeb..24aad0d 100644
--- a/lib/tst_safe_macros.c
+++ b/lib/tst_safe_macros.c
@@ -20,6 +20,25 @@
 #include "lapi/personality.h"
 #include "lapi/pidfd.h"
 
+int safe_access(const char *file, const int lineno,
+           const char *pathname, int mode)
+{
+        int rval;
+
+        rval = access(pathname, mode);
+
+        if (rval == -1) {
+                tst_brk_(file, lineno, TBROK | TERRNO,
+                        "access(%s,%d) failed", pathname, mode);
+        } else if (rval) {
+                tst_brk_(file, lineno, TBROK | TERRNO,
+                        "Invalid access(%s,%d) return value %d", pathname,
+                        mode, rval);
+        }
+
+        return rval;
+}
+
 int safe_setpgid(const char *file, const int lineno, pid_t pid, pid_t pgid)
 {
 	int rval;
-- 
1.8.3.1


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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [LTP] [PATCH v2 2/3] Make use of SAFE_ACCESS
  2022-03-17 21:00     ` [LTP] [PATCH v2 1/3] Add SAFE_ACCESS macro Dai Shili
@ 2022-03-17 21:00       ` Dai Shili
  2022-03-18  4:02         ` xuyang2018.jy
  2022-03-17 21:00       ` [LTP] [PATCH v2 3/3] syscalls/umount2: Convert to new API and use SAFE_ACCESS Dai Shili
  2022-03-18  3:56       ` [LTP] [PATCH v2 " xuyang2018.jy
  2 siblings, 1 reply; 27+ messages in thread
From: Dai Shili @ 2022-03-17 21:00 UTC (permalink / raw)
  To: xuyang2018.jy; +Cc: ltp

Signed-off-by: Dai Shili <daisl.fnst@fujitsu.com>
---
 testcases/kernel/mem/tunable/max_map_count.c    | 4 +---
 testcases/kernel/syscalls/fanotify/fanotify17.c | 3 +--
 testcases/kernel/syscalls/getxattr/getxattr05.c | 3 +--
 testcases/kernel/syscalls/madvise/madvise08.c   | 3 +--
 testcases/kernel/syscalls/quotactl/quotactl01.c | 6 ++----
 testcases/kernel/syscalls/quotactl/quotactl06.c | 3 +--
 6 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/testcases/kernel/mem/tunable/max_map_count.c b/testcases/kernel/mem/tunable/max_map_count.c
index a4c3dbf..caf8972 100644
--- a/testcases/kernel/mem/tunable/max_map_count.c
+++ b/testcases/kernel/mem/tunable/max_map_count.c
@@ -58,9 +58,7 @@ static long old_overcommit = -1;
 
 static void setup(void)
 {
-	if (access(PATH_SYSVM "max_map_count", F_OK) != 0)
-		tst_brk(TBROK | TERRNO,
-			 "Can't support to test max_map_count");
+	SAFE_ACCESS(PATH_SYSVM "max_map_count", F_OK);
 
 	old_max_map_count = get_sys_tune("max_map_count");
 	old_overcommit = get_sys_tune("overcommit_memory");
diff --git a/testcases/kernel/syscalls/fanotify/fanotify17.c b/testcases/kernel/syscalls/fanotify/fanotify17.c
index 35beb53..7d74b25 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify17.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify17.c
@@ -159,8 +159,7 @@ static void do_unshare(int map_root)
 		 * uid_map file should exist since Linux 3.8 because
 		 * it is available on Linux 3.5
 		 */
-		if (access(UID_MAP, F_OK))
-			tst_brk(TBROK, "file %s didn't exist", UID_MAP);
+		SAFE_ACCESS(UID_MAP, F_OK);
 
 		SAFE_FILE_PRINTF(UID_MAP, "%d %d %d", 0, 0, 1);
 	}
diff --git a/testcases/kernel/syscalls/getxattr/getxattr05.c b/testcases/kernel/syscalls/getxattr/getxattr05.c
index c0d339f..3e73cc5 100644
--- a/testcases/kernel/syscalls/getxattr/getxattr05.c
+++ b/testcases/kernel/syscalls/getxattr/getxattr05.c
@@ -100,8 +100,7 @@ static void do_unshare(int map_root)
 		/* uid_map file should exist since Linux 3.8 because
 		 * it is available on Linux 3.5
 		 */
-		if (access(UID_MAP, F_OK))
-			tst_brk(TBROK, "file %s didn't exist", UID_MAP);
+		SAFE_ACCESS(UID_MAP, F_OK);
 
 		SAFE_FILE_PRINTF(UID_MAP, "%d %d %d", 0, 0, 1);
 	}
diff --git a/testcases/kernel/syscalls/madvise/madvise08.c b/testcases/kernel/syscalls/madvise/madvise08.c
index 30f3604..10549f4 100644
--- a/testcases/kernel/syscalls/madvise/madvise08.c
+++ b/testcases/kernel/syscalls/madvise/madvise08.c
@@ -110,8 +110,7 @@ static int find_sequence(int pid)
 
 	snprintf(dumpname, 256, "dump-%d", pid);
 	tst_res(TINFO, "Dump file should be %s", dumpname);
-	if (access(dumpname, F_OK))
-		tst_brk(TBROK | TERRNO, "Dump file was not found.");
+	SAFE_ACCESS(dumpname, F_OK);
 
 	dfd = SAFE_OPEN(dumpname, O_RDONLY);
 
diff --git a/testcases/kernel/syscalls/quotactl/quotactl01.c b/testcases/kernel/syscalls/quotactl/quotactl01.c
index 63f6e91..56c4f2f 100644
--- a/testcases/kernel/syscalls/quotactl/quotactl01.c
+++ b/testcases/kernel/syscalls/quotactl/quotactl01.c
@@ -160,11 +160,9 @@ static void setup(void)
 	SAFE_CMD(cmd, NULL, NULL);
 	fmt_id = var->fmt_id;
 
-	if (access(USRPATH, F_OK) == -1)
-		tst_brk(TFAIL | TERRNO, "user quotafile didn't exist");
+	SAFE_ACCESS(USRPATH, F_OK);
 
-	if (access(GRPPATH, F_OK) == -1)
-		tst_brk(TFAIL | TERRNO, "group quotafile didn't exist");
+	SAFE_ACCESS(GRPPATH, F_OK);
 
 	tst_require_quota_support(tst_device->dev, fmt_id, usrpath);
 
diff --git a/testcases/kernel/syscalls/quotactl/quotactl06.c b/testcases/kernel/syscalls/quotactl/quotactl06.c
index feb4750..0ac099b 100644
--- a/testcases/kernel/syscalls/quotactl/quotactl06.c
+++ b/testcases/kernel/syscalls/quotactl/quotactl06.c
@@ -186,8 +186,7 @@ static void setup(void)
 	/* vfsv0 block limit 2^42, vfsv1 block limit 2^63 - 1 */
 	set_dqmax.dqb_bsoftlimit = tst_variant ? 0x20000000000000 : 0x100000000;
 
-	if (access(USRPATH, F_OK) == -1)
-		tst_brk(TBROK | TERRNO, "user quotafile didn't exist");
+	SAFE_ACCESS(USRPATH, F_OK);
 
 	tst_require_quota_support(tst_device->dev, fmt_id, usrpath);
 
-- 
1.8.3.1


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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [LTP] [PATCH v2 3/3] syscalls/umount2: Convert to new API and use SAFE_ACCESS
  2022-03-17 21:00     ` [LTP] [PATCH v2 1/3] Add SAFE_ACCESS macro Dai Shili
  2022-03-17 21:00       ` [LTP] [PATCH v2 2/3] Make use of SAFE_ACCESS Dai Shili
@ 2022-03-17 21:00       ` Dai Shili
  2022-03-18  4:03         ` xuyang2018.jy
  2022-03-18  3:56       ` [LTP] [PATCH v2 " xuyang2018.jy
  2 siblings, 1 reply; 27+ messages in thread
From: Dai Shili @ 2022-03-17 21:00 UTC (permalink / raw)
  To: xuyang2018.jy; +Cc: ltp

1. use TST_EXP_FAIL and TST_EXP_PASS macro
2. use SAFE macro
3. simplify verify operations

Signed-off-by: Dai Shili <daisl.fnst@fujitsu.com>
---
 testcases/kernel/syscalls/umount2/umount2.h    |   5 +-
 testcases/kernel/syscalls/umount2/umount2_02.c | 194 +++++++------------------
 testcases/kernel/syscalls/umount2/umount2_03.c | 167 +++++----------------
 3 files changed, 94 insertions(+), 272 deletions(-)

diff --git a/testcases/kernel/syscalls/umount2/umount2.h b/testcases/kernel/syscalls/umount2/umount2.h
index 65e4c24..d1e486e 100644
--- a/testcases/kernel/syscalls/umount2/umount2.h
+++ b/testcases/kernel/syscalls/umount2/umount2.h
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) 2016 Cyril Hrubis <chrubis@suse.cz>
  *
@@ -28,13 +29,13 @@ static inline int umount2_retry(const char *target, int flags)
 		if (ret == 0 || errno != EBUSY)
 			return ret;
 
-		tst_resm(TINFO, "umount('%s', %i) failed with EBUSY, try %2i...",
+		tst_res(TINFO, "umount('%s', %i) failed with EBUSY, try %2i...",
 			 target, flags, i);
 
 		usleep(100000);
 	}
 
-	tst_resm(TWARN, "Failed to umount('%s', %i) after 50 retries",
+	tst_res(TWARN, "Failed to umount('%s', %i) after 50 retries",
 	         target, flags);
 
 	errno = EBUSY;
diff --git a/testcases/kernel/syscalls/umount2/umount2_02.c b/testcases/kernel/syscalls/umount2/umount2_02.c
index 7d558fa..39f1608 100644
--- a/testcases/kernel/syscalls/umount2/umount2_02.c
+++ b/testcases/kernel/syscalls/umount2/umount2_02.c
@@ -1,182 +1,90 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * Copyright (c) 2015 Fujitsu Ltd.
+ * Copyright (c) 2015-2022 FUJITSU LIMITED. All rights reserved
  * Author: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- *
- * You should have received a copy of the GNU General Public License
- * alone with this program.
  */
 
 /*
- * DESCRIPTION
+ * [Description]
+ *
  *  Test for feature MNT_EXPIRE of umount2().
- *  "Mark the mount point as expired.If a mount point is not currently
- *   in use, then an initial call to umount2() with this flag fails with
- *   the error EAGAIN, but marks the mount point as expired. The mount
- *   point remains expired as long as it isn't accessed by any process.
- *   A second umount2() call specifying MNT_EXPIRE unmounts an expired
- *   mount point. This flag cannot be specified with either MNT_FORCE or
- *   MNT_DETACH. (fails with the error EINVAL)"
+ *
+ * - EINVAL when flag is specified with either MNT_FORCE or MNT_DETACH
+ * - EAGAIN when initial call to umount2(2) with MNT_EXPIRE
+ * - EAGAIN when umount2(2) with MNT_EXPIRE after access(2)
+ * - succeed when second call to umount2(2) with MNT_EXPIRE
  */
 
-#include <errno.h>
 #include <sys/mount.h>
-
-#include "test.h"
-#include "safe_macros.h"
 #include "lapi/mount.h"
-
+#include "tst_test.h"
 #include "umount2.h"
 
-#define DIR_MODE	(S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH)
-#define MNTPOINT	"mntpoint"
-
-static void setup(void);
-static void test_umount2(int i);
-static void verify_failure(int i);
-static void verify_success(int i);
-static void cleanup(void);
-
-static const char *device;
-static const char *fs_type;
+#define MNTPOINT        "mntpoint"
 
 static int mount_flag;
 
-static struct test_case_t {
+static struct tcase {
 	int flag;
 	int exp_errno;
 	int do_access;
 	const char *desc;
-} test_cases[] = {
+} tcases[] = {
 	{MNT_EXPIRE | MNT_FORCE, EINVAL, 0,
-		"umount2(2) with MNT_EXPIRE | MNT_FORCE expected EINVAL"},
+		"umount2() with MNT_EXPIRE | MNT_FORCE expected EINVAL"},
+
 	{MNT_EXPIRE | MNT_DETACH, EINVAL, 0,
-		"umount2(2) with MNT_EXPIRE | MNT_DETACH expected EINVAL"},
+		"umount2() with MNT_EXPIRE | MNT_DETACH expected EINVAL"},
+
 	{MNT_EXPIRE, EAGAIN, 0,
-		"initial call to umount2(2) with MNT_EXPIRE expected EAGAIN"},
+		"initial call to umount2() with MNT_EXPIRE expected EAGAIN"},
+
 	{MNT_EXPIRE, EAGAIN, 1,
-		"umount2(2) with MNT_EXPIRE after access(2) expected EAGAIN"},
+		"umount2() with MNT_EXPIRE after access() expected EAGAIN"},
+
 	{MNT_EXPIRE, 0, 0,
-		"second call to umount2(2) with MNT_EXPIRE expected success"},
+		"second call to umount2() with MNT_EXPIRE expected success"},
 };
 
-char *TCID = "umount2_02";
-int TST_TOTAL = ARRAY_SIZE(test_cases);
-
-int main(int ac, char **av)
+static void test_umount2(unsigned int n)
 {
-	int lc;
-	int tc;
+	struct tcase *tc = &tcases[n];
 
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-
-		SAFE_MOUNT(cleanup, device, MNTPOINT, fs_type, 0, NULL);
+	if (!mount_flag) {
+		SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, NULL);
 		mount_flag = 1;
-
-		for (tc = 0; tc < TST_TOTAL; tc++)
-			test_umount2(tc);
-
-		if (mount_flag) {
-			if (tst_umount(MNTPOINT))
-				tst_brkm(TBROK, cleanup, "umount() failed");
-			mount_flag = 0;
-		}
 	}
 
-	cleanup();
-	tst_exit();
-}
-
-static void setup(void)
-{
-	tst_require_root();
-
-	if ((tst_kvercmp(2, 6, 8)) < 0) {
-		tst_brkm(TCONF, NULL, "This test can only run on kernels "
-			 "that are 2.6.8 or higher");
-	}
-
-	tst_sig(NOFORK, DEF_HANDLER, NULL);
-
-	tst_tmpdir();
+	tst_res(TINFO, "Testing %s", tc->desc);
 
-	fs_type = tst_dev_fs_type();
-	device = tst_acquire_device(cleanup);
+	if (tc->do_access)
+		SAFE_ACCESS(MNTPOINT, F_OK);
 
-	if (!device)
-		tst_brkm(TCONF, cleanup, "Failed to obtain block device");
-
-	tst_mkfs(cleanup, device, fs_type, NULL, NULL);
-
-	SAFE_MKDIR(cleanup, MNTPOINT, DIR_MODE);
-
-	TEST_PAUSE;
-}
-
-static void test_umount2(int i)
-{
-	/* a new access removes the expired mark of the mount point */
-	if (test_cases[i].do_access) {
-		if (access(MNTPOINT, F_OK) == -1)
-			tst_brkm(TBROK | TERRNO, cleanup, "access(2) failed");
-	}
-
-	TEST(umount2_retry(MNTPOINT, test_cases[i].flag));
-
-	if (test_cases[i].exp_errno != 0)
-		verify_failure(i);
-	else
-		verify_success(i);
-}
-
-static void verify_failure(int i)
-{
-	if (TEST_RETURN == 0) {
-		tst_resm(TFAIL, "%s passed unexpectedly", test_cases[i].desc);
-		mount_flag = 0;
-		return;
-	}
-
-	if (TEST_ERRNO != test_cases[i].exp_errno) {
-		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
-			 test_cases[i].desc);
-		return;
-	}
-
-	tst_resm(TPASS | TTERRNO, "umount2(2) failed as expected");
-}
-
-static void verify_success(int i)
-{
-	if (TEST_RETURN != 0) {
-		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
-			 test_cases[i].desc);
-		return;
+	if (tc->exp_errno) {
+		TST_EXP_FAIL(umount2_retry(MNTPOINT, tc->flag), tc->exp_errno,
+			"umount2_retry(%s, %d)", MNTPOINT, tc->flag);
+		if (!TST_PASS)
+			mount_flag = 0;
+	} else {
+		TST_EXP_PASS(umount2_retry(MNTPOINT, tc->flag),
+			"umount2_retry(%s, %d)", MNTPOINT, tc->flag);
+		if (TST_PASS)
+			mount_flag = 0;
 	}
-
-	tst_resm(TPASS, "umount2(2) succeeded as expected");
-	mount_flag = 0;
 }
 
 static void cleanup(void)
 {
-	if (mount_flag && tst_umount(MNTPOINT))
-		tst_resm(TWARN | TERRNO, "Failed to unmount");
-
-	if (device)
-		tst_release_device(device);
-
-	tst_rmdir();
+	if (mount_flag)
+		SAFE_UMOUNT(MNTPOINT);
 }
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.needs_device = 1,
+	.format_device = 1,
+	.mntpoint = MNTPOINT,
+	.test = test_umount2,
+};
diff --git a/testcases/kernel/syscalls/umount2/umount2_03.c b/testcases/kernel/syscalls/umount2/umount2_03.c
index a8fddf6..f44ff79 100644
--- a/testcases/kernel/syscalls/umount2/umount2_03.c
+++ b/testcases/kernel/syscalls/umount2/umount2_03.c
@@ -1,167 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * Copyright (c) 2015 Fujitsu Ltd.
+ * Copyright (c) 2015-2022 FUJITSU LIMITED. All rights reserved
  * Author: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- *
- * You should have received a copy of the GNU General Public License
- * alone with this program.
  */
 
 /*
- * DESCRIPTION
+ * [Description]
+ *
  *  Test for feature UMOUNT_NOFOLLOW of umount2().
- *  "Don't dereference target if it is a symbolic link,
- *   and fails with the error EINVAL."
+ *
+ * - EINVAL when target is a symbolic link
+ * - succeed when target is a mount point
  */
 
-#include <errno.h>
 #include <sys/mount.h>
-
-#include "test.h"
-#include "safe_macros.h"
+#include "tst_test.h"
 #include "lapi/mount.h"
-
 #include "umount2.h"
 
-#define DIR_MODE	(S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH)
 #define MNTPOINT	"mntpoint"
 #define SYMLINK	"symlink"
 
-static void setup(void);
-static void test_umount2(int i);
-static void verify_failure(int i);
-static void verify_success(int i);
-static void cleanup(void);
-
-static const char *device;
-static const char *fs_type;
-
 static int mount_flag;
 
 static struct test_case_t {
 	const char *mntpoint;
 	int exp_errno;
 	const char *desc;
-} test_cases[] = {
+} tcase[] = {
 	{SYMLINK, EINVAL,
 		"umount2('symlink', UMOUNT_NOFOLLOW) expected EINVAL"},
 	{MNTPOINT, 0,
 		"umount2('mntpoint', UMOUNT_NOFOLLOW) expected success"},
 };
 
-char *TCID = "umount2_03";
-int TST_TOTAL = ARRAY_SIZE(test_cases);
-
-int main(int ac, char **av)
+static void test_umount2(unsigned int n)
 {
-	int lc;
-	int tc;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
+	struct test_case_t *tc = &tcase[n];
 
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-
-		for (tc = 0; tc < TST_TOTAL; tc++)
-			test_umount2(tc);
+	if (!mount_flag) {
+		SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, NULL);
+		mount_flag = 1;
 	}
 
-	cleanup();
-	tst_exit();
-}
-
-static void setup(void)
-{
-	tst_require_root();
-
-	if ((tst_kvercmp(2, 6, 34)) < 0) {
-		tst_brkm(TCONF, NULL, "This test can only run on kernels "
-			 "that are 2.6.34 or higher");
-	}
-
-	tst_sig(NOFORK, DEF_HANDLER, NULL);
-
-	tst_tmpdir();
-
-	fs_type = tst_dev_fs_type();
-	device = tst_acquire_device(cleanup);
-
-	if (!device)
-		tst_brkm(TCONF, cleanup, "Failed to obtain block device");
-
-	tst_mkfs(cleanup, device, fs_type, NULL, NULL);
-
-	SAFE_MKDIR(cleanup, MNTPOINT, DIR_MODE);
-
-	SAFE_SYMLINK(cleanup, MNTPOINT, SYMLINK);
-
-	TEST_PAUSE;
-}
-
-static void test_umount2(int i)
-{
-	SAFE_MOUNT(cleanup, device, MNTPOINT, fs_type, 0, NULL);
-	mount_flag = 1;
-
-	TEST(umount2_retry(test_cases[i].mntpoint, UMOUNT_NOFOLLOW));
-
-	if (test_cases[i].exp_errno != 0)
-		verify_failure(i);
-	else
-		verify_success(i);
-
-	if (mount_flag) {
-		if (tst_umount(MNTPOINT))
-			tst_brkm(TBROK, cleanup, "umount() failed");
-		mount_flag = 0;
+	tst_res(TINFO, "Testing %s", tc->desc);
+
+	if (tc->exp_errno) {
+		TST_EXP_FAIL(umount2_retry(tc->mntpoint, UMOUNT_NOFOLLOW), tc->exp_errno,
+			"umount2_retry(%s, %d)", tc->mntpoint, UMOUNT_NOFOLLOW);
+		if (!TST_PASS)
+			mount_flag = 0;
+	} else {
+		TST_EXP_PASS(umount2_retry(tc->mntpoint, UMOUNT_NOFOLLOW),
+			"umount2_retry(%s, %d)", tc->mntpoint, UMOUNT_NOFOLLOW);
+		if (TST_PASS)
+			mount_flag = 0;
 	}
 }
 
-static void verify_failure(int i)
-{
-	if (TEST_RETURN == 0) {
-		tst_resm(TFAIL, "%s passed unexpectedly", test_cases[i].desc);
-		mount_flag = 0;
-		return;
-	}
-
-	if (TEST_ERRNO != test_cases[i].exp_errno) {
-		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
-			 test_cases[i].desc);
-		return;
-	}
-
-	tst_resm(TPASS | TTERRNO, "umount2(2) failed as expected");
-}
-
-static void verify_success(int i)
+static void setup(void)
 {
-	if (TEST_RETURN != 0) {
-		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
-			 test_cases[i].desc);
-		return;
-	}
-
-	tst_resm(TPASS, "umount2(2) succeeded as expected");
-	mount_flag = 0;
+	SAFE_SYMLINK(MNTPOINT, SYMLINK);
 }
 
 static void cleanup(void)
 {
-	if (mount_flag && tst_umount(MNTPOINT))
-		tst_resm(TWARN | TERRNO, "Failed to unmount");
-
-	if (device)
-		tst_release_device(device);
-
-	tst_rmdir();
+	if (mount_flag)
+		SAFE_UMOUNT(MNTPOINT);
 }
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcase),
+	.needs_tmpdir = 1,
+	.setup = setup,
+	.cleanup = cleanup,
+	.format_device = 1,
+	.mntpoint = MNTPOINT,
+	.test = test_umount2,
+};
-- 
1.8.3.1


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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [LTP] [PATCH v2 1/3] Add SAFE_ACCESS macro
  2022-03-17 21:00     ` [LTP] [PATCH v2 1/3] Add SAFE_ACCESS macro Dai Shili
  2022-03-17 21:00       ` [LTP] [PATCH v2 2/3] Make use of SAFE_ACCESS Dai Shili
  2022-03-17 21:00       ` [LTP] [PATCH v2 3/3] syscalls/umount2: Convert to new API and use SAFE_ACCESS Dai Shili
@ 2022-03-18  3:56       ` xuyang2018.jy
  2 siblings, 0 replies; 27+ messages in thread
From: xuyang2018.jy @ 2022-03-18  3:56 UTC (permalink / raw)
  To: daisl.fnst; +Cc: ltp

Hi Dai

This patch looks good to me,
Reviewed-by:Yang Xu <xuyang2018.jy@fujitsu.com>

Best Regards
Yang Xu

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [LTP] [PATCH v2 2/3] Make use of SAFE_ACCESS
  2022-03-17 21:00       ` [LTP] [PATCH v2 2/3] Make use of SAFE_ACCESS Dai Shili
@ 2022-03-18  4:02         ` xuyang2018.jy
  0 siblings, 0 replies; 27+ messages in thread
From: xuyang2018.jy @ 2022-03-18  4:02 UTC (permalink / raw)
  To: daisl.fnst; +Cc: ltp

Hi Dai

Looks good to me,
Reviewed-by: Yang Xu <xuyang2018.jy@fujitsu.com>

just some small nits:
Since quotactl01 use tst_brk(TFAIL | TERRNO), we should also
mention it ie commit 706d412a quotactl06: Replace tst_brk(TFAIL) with
TBROK does.

Also this patch need rebase since commit 26d117f8 ("syscalls/quotactl:
Remove tst_require_quota_support") break this source code.


Best Regards
Yang Xu
> Signed-off-by: Dai Shili<daisl.fnst@fujitsu.com>
> ---
>   testcases/kernel/mem/tunable/max_map_count.c    | 4 +---
>   testcases/kernel/syscalls/fanotify/fanotify17.c | 3 +--
>   testcases/kernel/syscalls/getxattr/getxattr05.c | 3 +--
>   testcases/kernel/syscalls/madvise/madvise08.c   | 3 +--
>   testcases/kernel/syscalls/quotactl/quotactl01.c | 6 ++----
>   testcases/kernel/syscalls/quotactl/quotactl06.c | 3 +--
>   6 files changed, 7 insertions(+), 15 deletions(-)
> 
> diff --git a/testcases/kernel/mem/tunable/max_map_count.c b/testcases/kernel/mem/tunable/max_map_count.c
> index a4c3dbf..caf8972 100644
> --- a/testcases/kernel/mem/tunable/max_map_count.c
> +++ b/testcases/kernel/mem/tunable/max_map_count.c
> @@ -58,9 +58,7 @@ static long old_overcommit = -1;
> 
>   static void setup(void)
>   {
> -	if (access(PATH_SYSVM "max_map_count", F_OK) != 0)
> -		tst_brk(TBROK | TERRNO,
> -			 "Can't support to test max_map_count");
> +	SAFE_ACCESS(PATH_SYSVM "max_map_count", F_OK);
> 
>   	old_max_map_count = get_sys_tune("max_map_count");
>   	old_overcommit = get_sys_tune("overcommit_memory");
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify17.c b/testcases/kernel/syscalls/fanotify/fanotify17.c
> index 35beb53..7d74b25 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify17.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify17.c
> @@ -159,8 +159,7 @@ static void do_unshare(int map_root)
>   		 * uid_map file should exist since Linux 3.8 because
>   		 * it is available on Linux 3.5
>   		 */
> -		if (access(UID_MAP, F_OK))
> -			tst_brk(TBROK, "file %s didn't exist", UID_MAP);
> +		SAFE_ACCESS(UID_MAP, F_OK);
> 
>   		SAFE_FILE_PRINTF(UID_MAP, "%d %d %d", 0, 0, 1);
>   	}
> diff --git a/testcases/kernel/syscalls/getxattr/getxattr05.c b/testcases/kernel/syscalls/getxattr/getxattr05.c
> index c0d339f..3e73cc5 100644
> --- a/testcases/kernel/syscalls/getxattr/getxattr05.c
> +++ b/testcases/kernel/syscalls/getxattr/getxattr05.c
> @@ -100,8 +100,7 @@ static void do_unshare(int map_root)
>   		/* uid_map file should exist since Linux 3.8 because
>   		 * it is available on Linux 3.5
>   		 */
> -		if (access(UID_MAP, F_OK))
> -			tst_brk(TBROK, "file %s didn't exist", UID_MAP);
> +		SAFE_ACCESS(UID_MAP, F_OK);
> 
>   		SAFE_FILE_PRINTF(UID_MAP, "%d %d %d", 0, 0, 1);
>   	}
> diff --git a/testcases/kernel/syscalls/madvise/madvise08.c b/testcases/kernel/syscalls/madvise/madvise08.c
> index 30f3604..10549f4 100644
> --- a/testcases/kernel/syscalls/madvise/madvise08.c
> +++ b/testcases/kernel/syscalls/madvise/madvise08.c
> @@ -110,8 +110,7 @@ static int find_sequence(int pid)
> 
>   	snprintf(dumpname, 256, "dump-%d", pid);
>   	tst_res(TINFO, "Dump file should be %s", dumpname);
> -	if (access(dumpname, F_OK))
> -		tst_brk(TBROK | TERRNO, "Dump file was not found.");
> +	SAFE_ACCESS(dumpname, F_OK);
> 
>   	dfd = SAFE_OPEN(dumpname, O_RDONLY);
> 
> diff --git a/testcases/kernel/syscalls/quotactl/quotactl01.c b/testcases/kernel/syscalls/quotactl/quotactl01.c
> index 63f6e91..56c4f2f 100644
> --- a/testcases/kernel/syscalls/quotactl/quotactl01.c
> +++ b/testcases/kernel/syscalls/quotactl/quotactl01.c
> @@ -160,11 +160,9 @@ static void setup(void)
>   	SAFE_CMD(cmd, NULL, NULL);
>   	fmt_id = var->fmt_id;
> 
> -	if (access(USRPATH, F_OK) == -1)
> -		tst_brk(TFAIL | TERRNO, "user quotafile didn't exist");
> +	SAFE_ACCESS(USRPATH, F_OK);
> 
> -	if (access(GRPPATH, F_OK) == -1)
> -		tst_brk(TFAIL | TERRNO, "group quotafile didn't exist");
> +	SAFE_ACCESS(GRPPATH, F_OK);
> 
>   	tst_require_quota_support(tst_device->dev, fmt_id, usrpath);
> 
> diff --git a/testcases/kernel/syscalls/quotactl/quotactl06.c b/testcases/kernel/syscalls/quotactl/quotactl06.c
> index feb4750..0ac099b 100644
> --- a/testcases/kernel/syscalls/quotactl/quotactl06.c
> +++ b/testcases/kernel/syscalls/quotactl/quotactl06.c
> @@ -186,8 +186,7 @@ static void setup(void)
>   	/* vfsv0 block limit 2^42, vfsv1 block limit 2^63 - 1 */
>   	set_dqmax.dqb_bsoftlimit = tst_variant ? 0x20000000000000 : 0x100000000;
> 
> -	if (access(USRPATH, F_OK) == -1)
> -		tst_brk(TBROK | TERRNO, "user quotafile didn't exist");
> +	SAFE_ACCESS(USRPATH, F_OK);
> 
>   	tst_require_quota_support(tst_device->dev, fmt_id, usrpath);
> 

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [LTP] [PATCH v2 3/3] syscalls/umount2: Convert to new API and use SAFE_ACCESS
  2022-03-17 21:00       ` [LTP] [PATCH v2 3/3] syscalls/umount2: Convert to new API and use SAFE_ACCESS Dai Shili
@ 2022-03-18  4:03         ` xuyang2018.jy
  2022-03-18  5:11           ` xuyang2018.jy
  0 siblings, 1 reply; 27+ messages in thread
From: xuyang2018.jy @ 2022-03-18  4:03 UTC (permalink / raw)
  To: daisl.fnst; +Cc: ltp

Hi Dai

Since umount2_02 and umount2_03 is similar, can we merge them into a
single case?

Best Regards
Yang Xu
> 1. use TST_EXP_FAIL and TST_EXP_PASS macro
> 2. use SAFE macro
> 3. simplify verify operations
> 
> Signed-off-by: Dai Shili<daisl.fnst@fujitsu.com>
> ---
>   testcases/kernel/syscalls/umount2/umount2.h    |   5 +-
>   testcases/kernel/syscalls/umount2/umount2_02.c | 194 +++++++------------------
>   testcases/kernel/syscalls/umount2/umount2_03.c | 167 +++++----------------
>   3 files changed, 94 insertions(+), 272 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/umount2/umount2.h b/testcases/kernel/syscalls/umount2/umount2.h
> index 65e4c24..d1e486e 100644
> --- a/testcases/kernel/syscalls/umount2/umount2.h
> +++ b/testcases/kernel/syscalls/umount2/umount2.h
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
>   /*
>    * Copyright (c) 2016 Cyril Hrubis<chrubis@suse.cz>
>    *
> @@ -28,13 +29,13 @@ static inline int umount2_retry(const char *target, int flags)
>   		if (ret == 0 || errno != EBUSY)
>   			return ret;
> 
> -		tst_resm(TINFO, "umount('%s', %i) failed with EBUSY, try %2i...",
> +		tst_res(TINFO, "umount('%s', %i) failed with EBUSY, try %2i...",
>   			 target, flags, i);
> 
>   		usleep(100000);
>   	}
> 
> -	tst_resm(TWARN, "Failed to umount('%s', %i) after 50 retries",
> +	tst_res(TWARN, "Failed to umount('%s', %i) after 50 retries",
>   	         target, flags);
> 
>   	errno = EBUSY;
> diff --git a/testcases/kernel/syscalls/umount2/umount2_02.c b/testcases/kernel/syscalls/umount2/umount2_02.c
> index 7d558fa..39f1608 100644
> --- a/testcases/kernel/syscalls/umount2/umount2_02.c
> +++ b/testcases/kernel/syscalls/umount2/umount2_02.c
> @@ -1,182 +1,90 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
>   /*
> - * Copyright (c) 2015 Fujitsu Ltd.
> + * Copyright (c) 2015-2022 FUJITSU LIMITED. All rights reserved
>    * Author: Guangwen Feng<fenggw-fnst@cn.fujitsu.com>
> - *
> - * This program is free software; you can redistribute it and/or modify it
> - * under the terms of version 2 of the GNU General Public License as
> - * published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it would be useful, but
> - * WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> - *
> - * You should have received a copy of the GNU General Public License
> - * alone with this program.
>    */
> 
>   /*
> - * DESCRIPTION
> + * [Description]
> + *
>    *  Test for feature MNT_EXPIRE of umount2().
> - *  "Mark the mount point as expired.If a mount point is not currently
> - *   in use, then an initial call to umount2() with this flag fails with
> - *   the error EAGAIN, but marks the mount point as expired. The mount
> - *   point remains expired as long as it isn't accessed by any process.
> - *   A second umount2() call specifying MNT_EXPIRE unmounts an expired
> - *   mount point. This flag cannot be specified with either MNT_FORCE or
> - *   MNT_DETACH. (fails with the error EINVAL)"
> + *
> + * - EINVAL when flag is specified with either MNT_FORCE or MNT_DETACH
> + * - EAGAIN when initial call to umount2(2) with MNT_EXPIRE
> + * - EAGAIN when umount2(2) with MNT_EXPIRE after access(2)
> + * - succeed when second call to umount2(2) with MNT_EXPIRE
>    */
> 
> -#include<errno.h>
>   #include<sys/mount.h>
> -
> -#include "test.h"
> -#include "safe_macros.h"
>   #include "lapi/mount.h"
> -
> +#include "tst_test.h"
>   #include "umount2.h"
> 
> -#define DIR_MODE	(S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH)
> -#define MNTPOINT	"mntpoint"
> -
> -static void setup(void);
> -static void test_umount2(int i);
> -static void verify_failure(int i);
> -static void verify_success(int i);
> -static void cleanup(void);
> -
> -static const char *device;
> -static const char *fs_type;
> +#define MNTPOINT        "mntpoint"
> 
>   static int mount_flag;
> 
> -static struct test_case_t {
> +static struct tcase {
>   	int flag;
>   	int exp_errno;
>   	int do_access;
>   	const char *desc;
> -} test_cases[] = {
> +} tcases[] = {
>   	{MNT_EXPIRE | MNT_FORCE, EINVAL, 0,
> -		"umount2(2) with MNT_EXPIRE | MNT_FORCE expected EINVAL"},
> +		"umount2() with MNT_EXPIRE | MNT_FORCE expected EINVAL"},
> +
>   	{MNT_EXPIRE | MNT_DETACH, EINVAL, 0,
> -		"umount2(2) with MNT_EXPIRE | MNT_DETACH expected EINVAL"},
> +		"umount2() with MNT_EXPIRE | MNT_DETACH expected EINVAL"},
> +
>   	{MNT_EXPIRE, EAGAIN, 0,
> -		"initial call to umount2(2) with MNT_EXPIRE expected EAGAIN"},
> +		"initial call to umount2() with MNT_EXPIRE expected EAGAIN"},
> +
>   	{MNT_EXPIRE, EAGAIN, 1,
> -		"umount2(2) with MNT_EXPIRE after access(2) expected EAGAIN"},
> +		"umount2() with MNT_EXPIRE after access() expected EAGAIN"},
> +
>   	{MNT_EXPIRE, 0, 0,
> -		"second call to umount2(2) with MNT_EXPIRE expected success"},
> +		"second call to umount2() with MNT_EXPIRE expected success"},
>   };
> 
> -char *TCID = "umount2_02";
> -int TST_TOTAL = ARRAY_SIZE(test_cases);
> -
> -int main(int ac, char **av)
> +static void test_umount2(unsigned int n)
>   {
> -	int lc;
> -	int tc;
> +	struct tcase *tc =&tcases[n];
> 
> -	tst_parse_opts(ac, av, NULL, NULL);
> -
> -	setup();
> -
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -		tst_count = 0;
> -
> -		SAFE_MOUNT(cleanup, device, MNTPOINT, fs_type, 0, NULL);
> +	if (!mount_flag) {
> +		SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, NULL);
>   		mount_flag = 1;
> -
> -		for (tc = 0; tc<  TST_TOTAL; tc++)
> -			test_umount2(tc);
> -
> -		if (mount_flag) {
> -			if (tst_umount(MNTPOINT))
> -				tst_brkm(TBROK, cleanup, "umount() failed");
> -			mount_flag = 0;
> -		}
>   	}
> 
> -	cleanup();
> -	tst_exit();
> -}
> -
> -static void setup(void)
> -{
> -	tst_require_root();
> -
> -	if ((tst_kvercmp(2, 6, 8))<  0) {
> -		tst_brkm(TCONF, NULL, "This test can only run on kernels "
> -			 "that are 2.6.8 or higher");
> -	}
> -
> -	tst_sig(NOFORK, DEF_HANDLER, NULL);
> -
> -	tst_tmpdir();
> +	tst_res(TINFO, "Testing %s", tc->desc);
> 
> -	fs_type = tst_dev_fs_type();
> -	device = tst_acquire_device(cleanup);
> +	if (tc->do_access)
> +		SAFE_ACCESS(MNTPOINT, F_OK);
> 
> -	if (!device)
> -		tst_brkm(TCONF, cleanup, "Failed to obtain block device");
> -
> -	tst_mkfs(cleanup, device, fs_type, NULL, NULL);
> -
> -	SAFE_MKDIR(cleanup, MNTPOINT, DIR_MODE);
> -
> -	TEST_PAUSE;
> -}
> -
> -static void test_umount2(int i)
> -{
> -	/* a new access removes the expired mark of the mount point */
> -	if (test_cases[i].do_access) {
> -		if (access(MNTPOINT, F_OK) == -1)
> -			tst_brkm(TBROK | TERRNO, cleanup, "access(2) failed");
> -	}
> -
> -	TEST(umount2_retry(MNTPOINT, test_cases[i].flag));
> -
> -	if (test_cases[i].exp_errno != 0)
> -		verify_failure(i);
> -	else
> -		verify_success(i);
> -}
> -
> -static void verify_failure(int i)
> -{
> -	if (TEST_RETURN == 0) {
> -		tst_resm(TFAIL, "%s passed unexpectedly", test_cases[i].desc);
> -		mount_flag = 0;
> -		return;
> -	}
> -
> -	if (TEST_ERRNO != test_cases[i].exp_errno) {
> -		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
> -			 test_cases[i].desc);
> -		return;
> -	}
> -
> -	tst_resm(TPASS | TTERRNO, "umount2(2) failed as expected");
> -}
> -
> -static void verify_success(int i)
> -{
> -	if (TEST_RETURN != 0) {
> -		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
> -			 test_cases[i].desc);
> -		return;
> +	if (tc->exp_errno) {
> +		TST_EXP_FAIL(umount2_retry(MNTPOINT, tc->flag), tc->exp_errno,
> +			"umount2_retry(%s, %d)", MNTPOINT, tc->flag);
> +		if (!TST_PASS)
> +			mount_flag = 0;
> +	} else {
> +		TST_EXP_PASS(umount2_retry(MNTPOINT, tc->flag),
> +			"umount2_retry(%s, %d)", MNTPOINT, tc->flag);
> +		if (TST_PASS)
> +			mount_flag = 0;
>   	}
> -
> -	tst_resm(TPASS, "umount2(2) succeeded as expected");
> -	mount_flag = 0;
>   }
> 
>   static void cleanup(void)
>   {
> -	if (mount_flag&&  tst_umount(MNTPOINT))
> -		tst_resm(TWARN | TERRNO, "Failed to unmount");
> -
> -	if (device)
> -		tst_release_device(device);
> -
> -	tst_rmdir();
> +	if (mount_flag)
> +		SAFE_UMOUNT(MNTPOINT);
>   }
> +
> +static struct tst_test test = {
> +	.tcnt = ARRAY_SIZE(tcases),
> +	.cleanup = cleanup,
> +	.needs_root = 1,
> +	.needs_device = 1,
> +	.format_device = 1,
> +	.mntpoint = MNTPOINT,
> +	.test = test_umount2,
> +};
> diff --git a/testcases/kernel/syscalls/umount2/umount2_03.c b/testcases/kernel/syscalls/umount2/umount2_03.c
> index a8fddf6..f44ff79 100644
> --- a/testcases/kernel/syscalls/umount2/umount2_03.c
> +++ b/testcases/kernel/syscalls/umount2/umount2_03.c
> @@ -1,167 +1,80 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
>   /*
> - * Copyright (c) 2015 Fujitsu Ltd.
> + * Copyright (c) 2015-2022 FUJITSU LIMITED. All rights reserved
>    * Author: Guangwen Feng<fenggw-fnst@cn.fujitsu.com>
> - *
> - * This program is free software; you can redistribute it and/or modify it
> - * under the terms of version 2 of the GNU General Public License as
> - * published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it would be useful, but
> - * WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> - *
> - * You should have received a copy of the GNU General Public License
> - * alone with this program.
>    */
> 
>   /*
> - * DESCRIPTION
> + * [Description]
> + *
>    *  Test for feature UMOUNT_NOFOLLOW of umount2().
> - *  "Don't dereference target if it is a symbolic link,
> - *   and fails with the error EINVAL."
> + *
> + * - EINVAL when target is a symbolic link
> + * - succeed when target is a mount point
>    */
> 
> -#include<errno.h>
>   #include<sys/mount.h>
> -
> -#include "test.h"
> -#include "safe_macros.h"
> +#include "tst_test.h"
>   #include "lapi/mount.h"
> -
>   #include "umount2.h"
> 
> -#define DIR_MODE	(S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH)
>   #define MNTPOINT	"mntpoint"
>   #define SYMLINK	"symlink"
> 
> -static void setup(void);
> -static void test_umount2(int i);
> -static void verify_failure(int i);
> -static void verify_success(int i);
> -static void cleanup(void);
> -
> -static const char *device;
> -static const char *fs_type;
> -
>   static int mount_flag;
> 
>   static struct test_case_t {
>   	const char *mntpoint;
>   	int exp_errno;
>   	const char *desc;
> -} test_cases[] = {
> +} tcase[] = {
>   	{SYMLINK, EINVAL,
>   		"umount2('symlink', UMOUNT_NOFOLLOW) expected EINVAL"},
>   	{MNTPOINT, 0,
>   		"umount2('mntpoint', UMOUNT_NOFOLLOW) expected success"},
>   };
> 
> -char *TCID = "umount2_03";
> -int TST_TOTAL = ARRAY_SIZE(test_cases);
> -
> -int main(int ac, char **av)
> +static void test_umount2(unsigned int n)
>   {
> -	int lc;
> -	int tc;
> -
> -	tst_parse_opts(ac, av, NULL, NULL);
> -
> -	setup();
> +	struct test_case_t *tc =&tcase[n];
> 
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -		tst_count = 0;
> -
> -		for (tc = 0; tc<  TST_TOTAL; tc++)
> -			test_umount2(tc);
> +	if (!mount_flag) {
> +		SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, NULL);
> +		mount_flag = 1;
>   	}
> 
> -	cleanup();
> -	tst_exit();
> -}
> -
> -static void setup(void)
> -{
> -	tst_require_root();
> -
> -	if ((tst_kvercmp(2, 6, 34))<  0) {
> -		tst_brkm(TCONF, NULL, "This test can only run on kernels "
> -			 "that are 2.6.34 or higher");
> -	}
> -
> -	tst_sig(NOFORK, DEF_HANDLER, NULL);
> -
> -	tst_tmpdir();
> -
> -	fs_type = tst_dev_fs_type();
> -	device = tst_acquire_device(cleanup);
> -
> -	if (!device)
> -		tst_brkm(TCONF, cleanup, "Failed to obtain block device");
> -
> -	tst_mkfs(cleanup, device, fs_type, NULL, NULL);
> -
> -	SAFE_MKDIR(cleanup, MNTPOINT, DIR_MODE);
> -
> -	SAFE_SYMLINK(cleanup, MNTPOINT, SYMLINK);
> -
> -	TEST_PAUSE;
> -}
> -
> -static void test_umount2(int i)
> -{
> -	SAFE_MOUNT(cleanup, device, MNTPOINT, fs_type, 0, NULL);
> -	mount_flag = 1;
> -
> -	TEST(umount2_retry(test_cases[i].mntpoint, UMOUNT_NOFOLLOW));
> -
> -	if (test_cases[i].exp_errno != 0)
> -		verify_failure(i);
> -	else
> -		verify_success(i);
> -
> -	if (mount_flag) {
> -		if (tst_umount(MNTPOINT))
> -			tst_brkm(TBROK, cleanup, "umount() failed");
> -		mount_flag = 0;
> +	tst_res(TINFO, "Testing %s", tc->desc);
> +
> +	if (tc->exp_errno) {
> +		TST_EXP_FAIL(umount2_retry(tc->mntpoint, UMOUNT_NOFOLLOW), tc->exp_errno,
> +			"umount2_retry(%s, %d)", tc->mntpoint, UMOUNT_NOFOLLOW);
> +		if (!TST_PASS)
> +			mount_flag = 0;
> +	} else {
> +		TST_EXP_PASS(umount2_retry(tc->mntpoint, UMOUNT_NOFOLLOW),
> +			"umount2_retry(%s, %d)", tc->mntpoint, UMOUNT_NOFOLLOW);
> +		if (TST_PASS)
> +			mount_flag = 0;
>   	}
>   }
> 
> -static void verify_failure(int i)
> -{
> -	if (TEST_RETURN == 0) {
> -		tst_resm(TFAIL, "%s passed unexpectedly", test_cases[i].desc);
> -		mount_flag = 0;
> -		return;
> -	}
> -
> -	if (TEST_ERRNO != test_cases[i].exp_errno) {
> -		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
> -			 test_cases[i].desc);
> -		return;
> -	}
> -
> -	tst_resm(TPASS | TTERRNO, "umount2(2) failed as expected");
> -}
> -
> -static void verify_success(int i)
> +static void setup(void)
>   {
> -	if (TEST_RETURN != 0) {
> -		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
> -			 test_cases[i].desc);
> -		return;
> -	}
> -
> -	tst_resm(TPASS, "umount2(2) succeeded as expected");
> -	mount_flag = 0;
> +	SAFE_SYMLINK(MNTPOINT, SYMLINK);
>   }
> 
>   static void cleanup(void)
>   {
> -	if (mount_flag&&  tst_umount(MNTPOINT))
> -		tst_resm(TWARN | TERRNO, "Failed to unmount");
> -
> -	if (device)
> -		tst_release_device(device);
> -
> -	tst_rmdir();
> +	if (mount_flag)
> +		SAFE_UMOUNT(MNTPOINT);
>   }
> +
> +static struct tst_test test = {
> +	.tcnt = ARRAY_SIZE(tcase),
> +	.needs_tmpdir = 1,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.format_device = 1,
> +	.mntpoint = MNTPOINT,
> +	.test = test_umount2,
> +};

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [LTP] [PATCH v2 3/3] syscalls/umount2: Convert to new API and use SAFE_ACCESS
  2022-03-18  4:03         ` xuyang2018.jy
@ 2022-03-18  5:11           ` xuyang2018.jy
  2022-03-18 18:55             ` [LTP] [PATCH v3 1/3] Add SAFE_ACCESS macro Dai Shili
  0 siblings, 1 reply; 27+ messages in thread
From: xuyang2018.jy @ 2022-03-18  5:11 UTC (permalink / raw)
  To: daisl.fnst; +Cc: ltp

Hi Dai

Sorry, it seems keep umount2_02 and umount2_03 is more clean.
> Hi Dai
>
> Since umount2_02 and umount2_03 is similar, can we merge them into a
> single case?
>
> Best Regards
> Yang Xu
>> 1. use TST_EXP_FAIL and TST_EXP_PASS macro
>> 2. use SAFE macro
>> 3. simplify verify operations
>>
>> Signed-off-by: Dai Shili<daisl.fnst@fujitsu.com>
>> ---
>>    testcases/kernel/syscalls/umount2/umount2.h    |   5 +-
>>    testcases/kernel/syscalls/umount2/umount2_02.c | 194 +++++++------------------
>>    testcases/kernel/syscalls/umount2/umount2_03.c | 167 +++++----------------
>>    3 files changed, 94 insertions(+), 272 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/umount2/umount2.h b/testcases/kernel/syscalls/umount2/umount2.h
>> index 65e4c24..d1e486e 100644
>> --- a/testcases/kernel/syscalls/umount2/umount2.h
>> +++ b/testcases/kernel/syscalls/umount2/umount2.h
>> @@ -1,3 +1,4 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>    /*
>>     * Copyright (c) 2016 Cyril Hrubis<chrubis@suse.cz>
>>     *
>> @@ -28,13 +29,13 @@ static inline int umount2_retry(const char *target, int flags)
>>    		if (ret == 0 || errno != EBUSY)
>>    			return ret;
>>
>> -		tst_resm(TINFO, "umount('%s', %i) failed with EBUSY, try %2i...",
>> +		tst_res(TINFO, "umount('%s', %i) failed with EBUSY, try %2i...",
>>    			 target, flags, i);
>>
>>    		usleep(100000);
>>    	}
>>
>> -	tst_resm(TWARN, "Failed to umount('%s', %i) after 50 retries",
>> +	tst_res(TWARN, "Failed to umount('%s', %i) after 50 retries",
>>    	         target, flags);
>>
>>    	errno = EBUSY;
>> diff --git a/testcases/kernel/syscalls/umount2/umount2_02.c b/testcases/kernel/syscalls/umount2/umount2_02.c
>> index 7d558fa..39f1608 100644
>> --- a/testcases/kernel/syscalls/umount2/umount2_02.c
>> +++ b/testcases/kernel/syscalls/umount2/umount2_02.c
>> @@ -1,182 +1,90 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>    /*
>> - * Copyright (c) 2015 Fujitsu Ltd.
>> + * Copyright (c) 2015-2022 FUJITSU LIMITED. All rights reserved
>>     * Author: Guangwen Feng<fenggw-fnst@cn.fujitsu.com>
>> - *
>> - * This program is free software; you can redistribute it and/or modify it
>> - * under the terms of version 2 of the GNU General Public License as
>> - * published by the Free Software Foundation.
>> - *
>> - * This program is distributed in the hope that it would be useful, but
>> - * WITHOUT ANY WARRANTY; without even the implied warranty of
>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>> - *
>> - * You should have received a copy of the GNU General Public License
>> - * alone with this program.
>>     */
>>
>>    /*
typo, /* => /*\
>> - * DESCRIPTION
>> + * [Description]
>> + *
>>     *  Test for feature MNT_EXPIRE of umount2().
>> - *  "Mark the mount point as expired.If a mount point is not currently
>> - *   in use, then an initial call to umount2() with this flag fails with
>> - *   the error EAGAIN, but marks the mount point as expired. The mount
>> - *   point remains expired as long as it isn't accessed by any process.
>> - *   A second umount2() call specifying MNT_EXPIRE unmounts an expired
>> - *   mount point. This flag cannot be specified with either MNT_FORCE or
>> - *   MNT_DETACH. (fails with the error EINVAL)"
>> + *
>> + * - EINVAL when flag is specified with either MNT_FORCE or MNT_DETACH
>> + * - EAGAIN when initial call to umount2(2) with MNT_EXPIRE
>> + * - EAGAIN when umount2(2) with MNT_EXPIRE after access(2)
>> + * - succeed when second call to umount2(2) with MNT_EXPIRE
>>     */
>>
>> -#include<errno.h>
>>    #include<sys/mount.h>
>> -
>> -#include "test.h"
>> -#include "safe_macros.h"
>>    #include "lapi/mount.h"
>> -
>> +#include "tst_test.h"
>>    #include "umount2.h"
>>
>> -#define DIR_MODE	(S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH)
>> -#define MNTPOINT	"mntpoint"
>> -
>> -static void setup(void);
>> -static void test_umount2(int i);
>> -static void verify_failure(int i);
>> -static void verify_success(int i);
>> -static void cleanup(void);
>> -
>> -static const char *device;
>> -static const char *fs_type;
>> +#define MNTPOINT        "mntpoint"
>>
>>    static int mount_flag;
>>
>> -static struct test_case_t {
>> +static struct tcase {
>>    	int flag;
>>    	int exp_errno;
>>    	int do_access;
>>    	const char *desc;
>> -} test_cases[] = {
>> +} tcases[] = {
>>    	{MNT_EXPIRE | MNT_FORCE, EINVAL, 0,
>> -		"umount2(2) with MNT_EXPIRE | MNT_FORCE expected EINVAL"},
>> +		"umount2() with MNT_EXPIRE | MNT_FORCE expected EINVAL"},
>> +
>>    	{MNT_EXPIRE | MNT_DETACH, EINVAL, 0,
>> -		"umount2(2) with MNT_EXPIRE | MNT_DETACH expected EINVAL"},
>> +		"umount2() with MNT_EXPIRE | MNT_DETACH expected EINVAL"},
>> +
>>    	{MNT_EXPIRE, EAGAIN, 0,
>> -		"initial call to umount2(2) with MNT_EXPIRE expected EAGAIN"},
>> +		"initial call to umount2() with MNT_EXPIRE expected EAGAIN"},
>> +
>>    	{MNT_EXPIRE, EAGAIN, 1,
>> -		"umount2(2) with MNT_EXPIRE after access(2) expected EAGAIN"},
>> +		"umount2() with MNT_EXPIRE after access() expected EAGAIN"},
>> +
>>    	{MNT_EXPIRE, 0, 0,
>> -		"second call to umount2(2) with MNT_EXPIRE expected success"},
>> +		"second call to umount2() with MNT_EXPIRE expected success"},
>>    };
>>
>> -char *TCID = "umount2_02";
>> -int TST_TOTAL = ARRAY_SIZE(test_cases);
>> -
>> -int main(int ac, char **av)
>> +static void test_umount2(unsigned int n)
>>    {
>> -	int lc;
>> -	int tc;
>> +	struct tcase *tc =&tcases[n];
>>
>> -	tst_parse_opts(ac, av, NULL, NULL);
>> -
>> -	setup();
>> -
>> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
>> -		tst_count = 0;
>> -
>> -		SAFE_MOUNT(cleanup, device, MNTPOINT, fs_type, 0, NULL);
>> +	if (!mount_flag) {
>> +		SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, NULL);
>>    		mount_flag = 1;
>> -
>> -		for (tc = 0; tc<   TST_TOTAL; tc++)
>> -			test_umount2(tc);
>> -
>> -		if (mount_flag) {
>> -			if (tst_umount(MNTPOINT))
>> -				tst_brkm(TBROK, cleanup, "umount() failed");
>> -			mount_flag = 0;
>> -		}
>>    	}
>>
>> -	cleanup();
>> -	tst_exit();
>> -}
>> -
>> -static void setup(void)
>> -{
>> -	tst_require_root();
>> -
>> -	if ((tst_kvercmp(2, 6, 8))<   0) {
>> -		tst_brkm(TCONF, NULL, "This test can only run on kernels "
>> -			 "that are 2.6.8 or higher");
>> -	}
>> -
>> -	tst_sig(NOFORK, DEF_HANDLER, NULL);
>> -
>> -	tst_tmpdir();
>> +	tst_res(TINFO, "Testing %s", tc->desc);
>>
>> -	fs_type = tst_dev_fs_type();
>> -	device = tst_acquire_device(cleanup);
>> +	if (tc->do_access)
>> +		SAFE_ACCESS(MNTPOINT, F_OK);
>>
>> -	if (!device)
>> -		tst_brkm(TCONF, cleanup, "Failed to obtain block device");
>> -
>> -	tst_mkfs(cleanup, device, fs_type, NULL, NULL);
>> -
>> -	SAFE_MKDIR(cleanup, MNTPOINT, DIR_MODE);
>> -
>> -	TEST_PAUSE;
>> -}
>> -
>> -static void test_umount2(int i)
>> -{
>> -	/* a new access removes the expired mark of the mount point */
>> -	if (test_cases[i].do_access) {
>> -		if (access(MNTPOINT, F_OK) == -1)
>> -			tst_brkm(TBROK | TERRNO, cleanup, "access(2) failed");
>> -	}
>> -
>> -	TEST(umount2_retry(MNTPOINT, test_cases[i].flag));
>> -
>> -	if (test_cases[i].exp_errno != 0)
>> -		verify_failure(i);
>> -	else
>> -		verify_success(i);
>> -}
>> -
>> -static void verify_failure(int i)
>> -{
>> -	if (TEST_RETURN == 0) {
>> -		tst_resm(TFAIL, "%s passed unexpectedly", test_cases[i].desc);
>> -		mount_flag = 0;
>> -		return;
>> -	}
>> -
>> -	if (TEST_ERRNO != test_cases[i].exp_errno) {
>> -		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
>> -			 test_cases[i].desc);
>> -		return;
>> -	}
>> -
>> -	tst_resm(TPASS | TTERRNO, "umount2(2) failed as expected");
>> -}
>> -
>> -static void verify_success(int i)
>> -{
>> -	if (TEST_RETURN != 0) {
>> -		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
>> -			 test_cases[i].desc);
>> -		return;
>> +	if (tc->exp_errno) {
>> +		TST_EXP_FAIL(umount2_retry(MNTPOINT, tc->flag), tc->exp_errno,
>> +			"umount2_retry(%s, %d)", MNTPOINT, tc->flag);
>> +		if (!TST_PASS)
>> +			mount_flag = 0;
>> +	} else {
>> +		TST_EXP_PASS(umount2_retry(MNTPOINT, tc->flag),
>> +			"umount2_retry(%s, %d)", MNTPOINT, tc->flag);
>> +		if (TST_PASS)
>> +			mount_flag = 0;
>>    	}
>> -
>> -	tst_resm(TPASS, "umount2(2) succeeded as expected");
>> -	mount_flag = 0;
>>    }
>>
>>    static void cleanup(void)
>>    {
>> -	if (mount_flag&&   tst_umount(MNTPOINT))
>> -		tst_resm(TWARN | TERRNO, "Failed to unmount");
>> -
>> -	if (device)
>> -		tst_release_device(device);
>> -
>> -	tst_rmdir();
>> +	if (mount_flag)
>> +		SAFE_UMOUNT(MNTPOINT);
>>    }
>> +
>> +static struct tst_test test = {
>> +	.tcnt = ARRAY_SIZE(tcases),
>> +	.cleanup = cleanup,
>> +	.needs_root = 1,
>> +	.needs_device = 1,
>> +	.format_device = 1,
If we specify format_device, it wil assign value for needs_device a 
utomatically. So we can remove redundant needs_device.
>> +	.mntpoint = MNTPOINT,
>> +	.test = test_umount2,
>> +};
>> diff --git a/testcases/kernel/syscalls/umount2/umount2_03.c b/testcases/kernel/syscalls/umount2/umount2_03.c
>> index a8fddf6..f44ff79 100644
>> --- a/testcases/kernel/syscalls/umount2/umount2_03.c
>> +++ b/testcases/kernel/syscalls/umount2/umount2_03.c
>> @@ -1,167 +1,80 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>    /*
>> - * Copyright (c) 2015 Fujitsu Ltd.
>> + * Copyright (c) 2015-2022 FUJITSU LIMITED. All rights reserved
>>     * Author: Guangwen Feng<fenggw-fnst@cn.fujitsu.com>
>> - *
>> - * This program is free software; you can redistribute it and/or modify it
>> - * under the terms of version 2 of the GNU General Public License as
>> - * published by the Free Software Foundation.
>> - *
>> - * This program is distributed in the hope that it would be useful, but
>> - * WITHOUT ANY WARRANTY; without even the implied warranty of
>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>> - *
>> - * You should have received a copy of the GNU General Public License
>> - * alone with this program.
>>     */
>>
>>    /*
Here as well.

Best Regards
Yang Xu
>> - * DESCRIPTION
>> + * [Description]
>> + *
>>     *  Test for feature UMOUNT_NOFOLLOW of umount2().
>> - *  "Don't dereference target if it is a symbolic link,
>> - *   and fails with the error EINVAL."
>> + *
>> + * - EINVAL when target is a symbolic link
>> + * - succeed when target is a mount point
>>     */
>>
>> -#include<errno.h>
>>    #include<sys/mount.h>
>> -
>> -#include "test.h"
>> -#include "safe_macros.h"
>> +#include "tst_test.h"
>>    #include "lapi/mount.h"
>> -
>>    #include "umount2.h"
>>
>> -#define DIR_MODE	(S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH)
>>    #define MNTPOINT	"mntpoint"
>>    #define SYMLINK	"symlink"
>>
>> -static void setup(void);
>> -static void test_umount2(int i);
>> -static void verify_failure(int i);
>> -static void verify_success(int i);
>> -static void cleanup(void);
>> -
>> -static const char *device;
>> -static const char *fs_type;
>> -
>>    static int mount_flag;
>>
>>    static struct test_case_t {
>>    	const char *mntpoint;
>>    	int exp_errno;
>>    	const char *desc;
>> -} test_cases[] = {
>> +} tcase[] = {
>>    	{SYMLINK, EINVAL,
>>    		"umount2('symlink', UMOUNT_NOFOLLOW) expected EINVAL"},
>>    	{MNTPOINT, 0,
>>    		"umount2('mntpoint', UMOUNT_NOFOLLOW) expected success"},
>>    };
>>
>> -char *TCID = "umount2_03";
>> -int TST_TOTAL = ARRAY_SIZE(test_cases);
>> -
>> -int main(int ac, char **av)
>> +static void test_umount2(unsigned int n)
>>    {
>> -	int lc;
>> -	int tc;
>> -
>> -	tst_parse_opts(ac, av, NULL, NULL);
>> -
>> -	setup();
>> +	struct test_case_t *tc =&tcase[n];
>>
>> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
>> -		tst_count = 0;
>> -
>> -		for (tc = 0; tc<   TST_TOTAL; tc++)
>> -			test_umount2(tc);
>> +	if (!mount_flag) {
>> +		SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, NULL);
>> +		mount_flag = 1;
>>    	}
>>
>> -	cleanup();
>> -	tst_exit();
>> -}
>> -
>> -static void setup(void)
>> -{
>> -	tst_require_root();
>> -
>> -	if ((tst_kvercmp(2, 6, 34))<   0) {
>> -		tst_brkm(TCONF, NULL, "This test can only run on kernels "
>> -			 "that are 2.6.34 or higher");
>> -	}
>> -
>> -	tst_sig(NOFORK, DEF_HANDLER, NULL);
>> -
>> -	tst_tmpdir();
>> -
>> -	fs_type = tst_dev_fs_type();
>> -	device = tst_acquire_device(cleanup);
>> -
>> -	if (!device)
>> -		tst_brkm(TCONF, cleanup, "Failed to obtain block device");
>> -
>> -	tst_mkfs(cleanup, device, fs_type, NULL, NULL);
>> -
>> -	SAFE_MKDIR(cleanup, MNTPOINT, DIR_MODE);
>> -
>> -	SAFE_SYMLINK(cleanup, MNTPOINT, SYMLINK);
>> -
>> -	TEST_PAUSE;
>> -}
>> -
>> -static void test_umount2(int i)
>> -{
>> -	SAFE_MOUNT(cleanup, device, MNTPOINT, fs_type, 0, NULL);
>> -	mount_flag = 1;
>> -
>> -	TEST(umount2_retry(test_cases[i].mntpoint, UMOUNT_NOFOLLOW));
>> -
>> -	if (test_cases[i].exp_errno != 0)
>> -		verify_failure(i);
>> -	else
>> -		verify_success(i);
>> -
>> -	if (mount_flag) {
>> -		if (tst_umount(MNTPOINT))
>> -			tst_brkm(TBROK, cleanup, "umount() failed");
>> -		mount_flag = 0;
>> +	tst_res(TINFO, "Testing %s", tc->desc);
>> +
>> +	if (tc->exp_errno) {
>> +		TST_EXP_FAIL(umount2_retry(tc->mntpoint, UMOUNT_NOFOLLOW), tc->exp_errno,
>> +			"umount2_retry(%s, %d)", tc->mntpoint, UMOUNT_NOFOLLOW);
>> +		if (!TST_PASS)
>> +			mount_flag = 0;
>> +	} else {
>> +		TST_EXP_PASS(umount2_retry(tc->mntpoint, UMOUNT_NOFOLLOW),
>> +			"umount2_retry(%s, %d)", tc->mntpoint, UMOUNT_NOFOLLOW);
>> +		if (TST_PASS)
>> +			mount_flag = 0;
>>    	}
>>    }
>>
>> -static void verify_failure(int i)
>> -{
>> -	if (TEST_RETURN == 0) {
>> -		tst_resm(TFAIL, "%s passed unexpectedly", test_cases[i].desc);
>> -		mount_flag = 0;
>> -		return;
>> -	}
>> -
>> -	if (TEST_ERRNO != test_cases[i].exp_errno) {
>> -		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
>> -			 test_cases[i].desc);
>> -		return;
>> -	}
>> -
>> -	tst_resm(TPASS | TTERRNO, "umount2(2) failed as expected");
>> -}
>> -
>> -static void verify_success(int i)
>> +static void setup(void)
>>    {
>> -	if (TEST_RETURN != 0) {
>> -		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
>> -			 test_cases[i].desc);
>> -		return;
>> -	}
>> -
>> -	tst_resm(TPASS, "umount2(2) succeeded as expected");
>> -	mount_flag = 0;
>> +	SAFE_SYMLINK(MNTPOINT, SYMLINK);
>>    }
>>
>>    static void cleanup(void)
>>    {
>> -	if (mount_flag&&   tst_umount(MNTPOINT))
>> -		tst_resm(TWARN | TERRNO, "Failed to unmount");
>> -
>> -	if (device)
>> -		tst_release_device(device);
>> -
>> -	tst_rmdir();
>> +	if (mount_flag)
>> +		SAFE_UMOUNT(MNTPOINT);
>>    }
>> +
>> +static struct tst_test test = {
>> +	.tcnt = ARRAY_SIZE(tcase),
>> +	.needs_tmpdir = 1,
>> +	.setup = setup,
>> +	.cleanup = cleanup,
>> +	.format_device = 1,
>> +	.mntpoint = MNTPOINT,
>> +	.test = test_umount2,
>> +};
>

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [LTP] [PATCH v3 1/3] Add SAFE_ACCESS macro
  2022-03-18  5:11           ` xuyang2018.jy
@ 2022-03-18 18:55             ` Dai Shili
  2022-03-18 18:55               ` [LTP] [PATCH v3 2/3] Make use of SAFE_ACCESS Dai Shili
                                 ` (4 more replies)
  0 siblings, 5 replies; 27+ messages in thread
From: Dai Shili @ 2022-03-18 18:55 UTC (permalink / raw)
  To: xuyang2018.jy; +Cc: ltp

Signed-off-by: Dai Shili <daisl.fnst@fujitsu.com>
---
 include/tst_safe_macros.h |  5 +++++
 lib/tst_safe_macros.c     | 19 +++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
index 9a543e4..81c4b08 100644
--- a/include/tst_safe_macros.h
+++ b/include/tst_safe_macros.h
@@ -24,6 +24,11 @@
 #include "safe_macros_fn.h"
 #include "tst_cmd.h"
 
+int safe_access(const char *filename, const int lineno, const char *pathname,
+		   int mode);
+#define SAFE_ACCESS(path, mode) \
+	safe_access(__FILE__, __LINE__, (path), (mode))
+
 #define SAFE_BASENAME(path) \
 	safe_basename(__FILE__, __LINE__, NULL, (path))
 
diff --git a/lib/tst_safe_macros.c b/lib/tst_safe_macros.c
index a9f6aeb..c4cdc87 100644
--- a/lib/tst_safe_macros.c
+++ b/lib/tst_safe_macros.c
@@ -20,6 +20,25 @@
 #include "lapi/personality.h"
 #include "lapi/pidfd.h"
 
+int safe_access(const char *file, const int lineno,
+	    const char *pathname, int mode)
+{
+	int rval;
+
+	rval = access(pathname, mode);
+
+	if (rval == -1) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"access(%s,%d) failed", pathname, mode);
+	} else if (rval) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"Invalid access(%s,%d) return value %d", pathname,
+			mode, rval);
+	}
+
+	return rval;
+}
+
 int safe_setpgid(const char *file, const int lineno, pid_t pid, pid_t pgid)
 {
 	int rval;
-- 
1.8.3.1


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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [LTP] [PATCH v3 2/3] Make use of SAFE_ACCESS
  2022-03-18 18:55             ` [LTP] [PATCH v3 1/3] Add SAFE_ACCESS macro Dai Shili
@ 2022-03-18 18:55               ` Dai Shili
  2022-03-23 19:30                 ` Petr Vorel
  2022-03-23 19:33                 ` Petr Vorel
  2022-03-18 18:55               ` [LTP] [PATCH v3 3/3] syscalls/umount2: Convert to new API and use SAFE_ACCESS Dai Shili
                                 ` (3 subsequent siblings)
  4 siblings, 2 replies; 27+ messages in thread
From: Dai Shili @ 2022-03-18 18:55 UTC (permalink / raw)
  To: xuyang2018.jy; +Cc: ltp

1. quotactl01: Replace tst_brk(TFAIL) with TBROK
2. use SAFE_ACCESS

Signed-off-by: Dai Shili <daisl.fnst@fujitsu.com>
---
 testcases/kernel/mem/tunable/max_map_count.c    | 4 +---
 testcases/kernel/syscalls/fanotify/fanotify17.c | 3 +--
 testcases/kernel/syscalls/getxattr/getxattr05.c | 3 +--
 testcases/kernel/syscalls/madvise/madvise08.c   | 3 +--
 testcases/kernel/syscalls/quotactl/quotactl01.c | 6 ++----
 testcases/kernel/syscalls/quotactl/quotactl06.c | 3 +--
 6 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/testcases/kernel/mem/tunable/max_map_count.c b/testcases/kernel/mem/tunable/max_map_count.c
index a4c3dbf..caf8972 100644
--- a/testcases/kernel/mem/tunable/max_map_count.c
+++ b/testcases/kernel/mem/tunable/max_map_count.c
@@ -58,9 +58,7 @@ static long old_overcommit = -1;
 
 static void setup(void)
 {
-	if (access(PATH_SYSVM "max_map_count", F_OK) != 0)
-		tst_brk(TBROK | TERRNO,
-			 "Can't support to test max_map_count");
+	SAFE_ACCESS(PATH_SYSVM "max_map_count", F_OK);
 
 	old_max_map_count = get_sys_tune("max_map_count");
 	old_overcommit = get_sys_tune("overcommit_memory");
diff --git a/testcases/kernel/syscalls/fanotify/fanotify17.c b/testcases/kernel/syscalls/fanotify/fanotify17.c
index 35beb53..7d74b25 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify17.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify17.c
@@ -159,8 +159,7 @@ static void do_unshare(int map_root)
 		 * uid_map file should exist since Linux 3.8 because
 		 * it is available on Linux 3.5
 		 */
-		if (access(UID_MAP, F_OK))
-			tst_brk(TBROK, "file %s didn't exist", UID_MAP);
+		SAFE_ACCESS(UID_MAP, F_OK);
 
 		SAFE_FILE_PRINTF(UID_MAP, "%d %d %d", 0, 0, 1);
 	}
diff --git a/testcases/kernel/syscalls/getxattr/getxattr05.c b/testcases/kernel/syscalls/getxattr/getxattr05.c
index c0d339f..3e73cc5 100644
--- a/testcases/kernel/syscalls/getxattr/getxattr05.c
+++ b/testcases/kernel/syscalls/getxattr/getxattr05.c
@@ -100,8 +100,7 @@ static void do_unshare(int map_root)
 		/* uid_map file should exist since Linux 3.8 because
 		 * it is available on Linux 3.5
 		 */
-		if (access(UID_MAP, F_OK))
-			tst_brk(TBROK, "file %s didn't exist", UID_MAP);
+		SAFE_ACCESS(UID_MAP, F_OK);
 
 		SAFE_FILE_PRINTF(UID_MAP, "%d %d %d", 0, 0, 1);
 	}
diff --git a/testcases/kernel/syscalls/madvise/madvise08.c b/testcases/kernel/syscalls/madvise/madvise08.c
index 30f3604..10549f4 100644
--- a/testcases/kernel/syscalls/madvise/madvise08.c
+++ b/testcases/kernel/syscalls/madvise/madvise08.c
@@ -110,8 +110,7 @@ static int find_sequence(int pid)
 
 	snprintf(dumpname, 256, "dump-%d", pid);
 	tst_res(TINFO, "Dump file should be %s", dumpname);
-	if (access(dumpname, F_OK))
-		tst_brk(TBROK | TERRNO, "Dump file was not found.");
+	SAFE_ACCESS(dumpname, F_OK);
 
 	dfd = SAFE_OPEN(dumpname, O_RDONLY);
 
diff --git a/testcases/kernel/syscalls/quotactl/quotactl01.c b/testcases/kernel/syscalls/quotactl/quotactl01.c
index c14228e..36ec93e 100644
--- a/testcases/kernel/syscalls/quotactl/quotactl01.c
+++ b/testcases/kernel/syscalls/quotactl/quotactl01.c
@@ -160,11 +160,9 @@ static void setup(void)
 	SAFE_CMD(cmd, NULL, NULL);
 	fmt_id = var->fmt_id;
 
-	if (access(USRPATH, F_OK) == -1)
-		tst_brk(TFAIL | TERRNO, "user quotafile didn't exist");
+	SAFE_ACCESS(USRPATH, F_OK);
 
-	if (access(GRPPATH, F_OK) == -1)
-		tst_brk(TFAIL | TERRNO, "group quotafile didn't exist");
+	SAFE_ACCESS(GRPPATH, F_OK);
 
 	TEST(quotactl(QCMD(Q_GETNEXTQUOTA, USRQUOTA), tst_device->dev,
 		test_id, (void *) &res_ndq));
diff --git a/testcases/kernel/syscalls/quotactl/quotactl06.c b/testcases/kernel/syscalls/quotactl/quotactl06.c
index ae8d7fa..74a098a 100644
--- a/testcases/kernel/syscalls/quotactl/quotactl06.c
+++ b/testcases/kernel/syscalls/quotactl/quotactl06.c
@@ -186,8 +186,7 @@ static void setup(void)
 	/* vfsv0 block limit 2^42, vfsv1 block limit 2^63 - 1 */
 	set_dqmax.dqb_bsoftlimit = tst_variant ? 0x20000000000000 : 0x100000000;
 
-	if (access(USRPATH, F_OK) == -1)
-		tst_brk(TBROK | TERRNO, "user quotafile didn't exist");
+	SAFE_ACCESS(USRPATH, F_OK);
 
 	SAFE_MKDIR(TESTDIR1, 0666);
 
-- 
1.8.3.1


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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [LTP] [PATCH v3 3/3] syscalls/umount2: Convert to new API and use SAFE_ACCESS
  2022-03-18 18:55             ` [LTP] [PATCH v3 1/3] Add SAFE_ACCESS macro Dai Shili
  2022-03-18 18:55               ` [LTP] [PATCH v3 2/3] Make use of SAFE_ACCESS Dai Shili
@ 2022-03-18 18:55               ` Dai Shili
  2022-03-23 19:36                 ` Petr Vorel
  2022-03-24  1:37                 ` [LTP] [PATCH v3 3/3] " xuyang2018.jy
  2022-03-23 19:24               ` [LTP] [PATCH v3 1/3] Add SAFE_ACCESS macro Petr Vorel
                                 ` (2 subsequent siblings)
  4 siblings, 2 replies; 27+ messages in thread
From: Dai Shili @ 2022-03-18 18:55 UTC (permalink / raw)
  To: xuyang2018.jy; +Cc: ltp

1. use TST_EXP_FAIL and TST_EXP_PASS macro
2. use SAFE macro
3. simplify verify operations
4. merge umount2_03 to umount2_02

Signed-off-by: Dai Shili <daisl.fnst@fujitsu.com>
---
 runtest/syscalls                               |   1 -
 testcases/kernel/syscalls/umount2/.gitignore   |   1 -
 testcases/kernel/syscalls/umount2/umount2.h    |   5 +-
 testcases/kernel/syscalls/umount2/umount2_02.c | 209 ++++++++-----------------
 testcases/kernel/syscalls/umount2/umount2_03.c | 167 --------------------
 5 files changed, 71 insertions(+), 312 deletions(-)
 delete mode 100644 testcases/kernel/syscalls/umount2/umount2_03.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 6186bfc..0eea946 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1647,7 +1647,6 @@ umount03 umount03
 
 umount2_01 umount2_01
 umount2_02 umount2_02
-umount2_03 umount2_03
 
 userfaultfd01 userfaultfd01
 
diff --git a/testcases/kernel/syscalls/umount2/.gitignore b/testcases/kernel/syscalls/umount2/.gitignore
index 1d6a0e8..e2056bf 100644
--- a/testcases/kernel/syscalls/umount2/.gitignore
+++ b/testcases/kernel/syscalls/umount2/.gitignore
@@ -1,3 +1,2 @@
 /umount2_01
 /umount2_02
-/umount2_03
diff --git a/testcases/kernel/syscalls/umount2/umount2.h b/testcases/kernel/syscalls/umount2/umount2.h
index 65e4c24..d1e486e 100644
--- a/testcases/kernel/syscalls/umount2/umount2.h
+++ b/testcases/kernel/syscalls/umount2/umount2.h
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) 2016 Cyril Hrubis <chrubis@suse.cz>
  *
@@ -28,13 +29,13 @@ static inline int umount2_retry(const char *target, int flags)
 		if (ret == 0 || errno != EBUSY)
 			return ret;
 
-		tst_resm(TINFO, "umount('%s', %i) failed with EBUSY, try %2i...",
+		tst_res(TINFO, "umount('%s', %i) failed with EBUSY, try %2i...",
 			 target, flags, i);
 
 		usleep(100000);
 	}
 
-	tst_resm(TWARN, "Failed to umount('%s', %i) after 50 retries",
+	tst_res(TWARN, "Failed to umount('%s', %i) after 50 retries",
 	         target, flags);
 
 	errno = EBUSY;
diff --git a/testcases/kernel/syscalls/umount2/umount2_02.c b/testcases/kernel/syscalls/umount2/umount2_02.c
index 7d558fa..8ac63a4 100644
--- a/testcases/kernel/syscalls/umount2/umount2_02.c
+++ b/testcases/kernel/syscalls/umount2/umount2_02.c
@@ -1,182 +1,109 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * Copyright (c) 2015 Fujitsu Ltd.
+ * Copyright (c) 2015-2022 FUJITSU LIMITED. All rights reserved
  * Author: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- *
- * You should have received a copy of the GNU General Public License
- * alone with this program.
  */
 
 /*
- * DESCRIPTION
+ * [Description]
+ *
  *  Test for feature MNT_EXPIRE of umount2().
- *  "Mark the mount point as expired.If a mount point is not currently
- *   in use, then an initial call to umount2() with this flag fails with
- *   the error EAGAIN, but marks the mount point as expired. The mount
- *   point remains expired as long as it isn't accessed by any process.
- *   A second umount2() call specifying MNT_EXPIRE unmounts an expired
- *   mount point. This flag cannot be specified with either MNT_FORCE or
- *   MNT_DETACH. (fails with the error EINVAL)"
+ *
+ * - EINVAL when flag is specified with either MNT_FORCE or MNT_DETACH
+ * - EAGAIN when initial call to umount2(2) with MNT_EXPIRE
+ * - EAGAIN when umount2(2) with MNT_EXPIRE after access(2)
+ * - succeed when second call to umount2(2) with MNT_EXPIRE
+ *
+ *  Test for feature UMOUNT_NOFOLLOW of umount2().
+ *
+ * - EINVAL when target is a symbolic link
+ * - succeed when target is a mount point
  */
 
-#include <errno.h>
 #include <sys/mount.h>
-
-#include "test.h"
-#include "safe_macros.h"
 #include "lapi/mount.h"
-
+#include "tst_test.h"
 #include "umount2.h"
 
-#define DIR_MODE	(S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH)
-#define MNTPOINT	"mntpoint"
-
-static void setup(void);
-static void test_umount2(int i);
-static void verify_failure(int i);
-static void verify_success(int i);
-static void cleanup(void);
-
-static const char *device;
-static const char *fs_type;
+#define MNTPOINT        "mntpoint"
+#define SYMLINK	"symlink"
 
 static int mount_flag;
 
-static struct test_case_t {
+static struct tcase {
+	const char *mntpoint;
 	int flag;
 	int exp_errno;
 	int do_access;
 	const char *desc;
-} test_cases[] = {
-	{MNT_EXPIRE | MNT_FORCE, EINVAL, 0,
-		"umount2(2) with MNT_EXPIRE | MNT_FORCE expected EINVAL"},
-	{MNT_EXPIRE | MNT_DETACH, EINVAL, 0,
-		"umount2(2) with MNT_EXPIRE | MNT_DETACH expected EINVAL"},
-	{MNT_EXPIRE, EAGAIN, 0,
-		"initial call to umount2(2) with MNT_EXPIRE expected EAGAIN"},
-	{MNT_EXPIRE, EAGAIN, 1,
-		"umount2(2) with MNT_EXPIRE after access(2) expected EAGAIN"},
-	{MNT_EXPIRE, 0, 0,
-		"second call to umount2(2) with MNT_EXPIRE expected success"},
-};
+} tcases[] = {
+	{MNTPOINT, MNT_EXPIRE | MNT_FORCE, EINVAL, 0,
+		"umount2() with MNT_EXPIRE | MNT_FORCE expected EINVAL"},
 
-char *TCID = "umount2_02";
-int TST_TOTAL = ARRAY_SIZE(test_cases);
+	{MNTPOINT, MNT_EXPIRE | MNT_DETACH, EINVAL, 0,
+		"umount2() with MNT_EXPIRE | MNT_DETACH expected EINVAL"},
 
-int main(int ac, char **av)
-{
-	int lc;
-	int tc;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
+	{MNTPOINT, MNT_EXPIRE, EAGAIN, 0,
+		"initial call to umount2() with MNT_EXPIRE expected EAGAIN"},
 
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
+	{MNTPOINT, MNT_EXPIRE, EAGAIN, 1,
+		"umount2() with MNT_EXPIRE after access() expected EAGAIN"},
 
-		SAFE_MOUNT(cleanup, device, MNTPOINT, fs_type, 0, NULL);
-		mount_flag = 1;
-
-		for (tc = 0; tc < TST_TOTAL; tc++)
-			test_umount2(tc);
+	{MNTPOINT, MNT_EXPIRE, 0, 0,
+		"second call to umount2() with MNT_EXPIRE expected success"},
 
-		if (mount_flag) {
-			if (tst_umount(MNTPOINT))
-				tst_brkm(TBROK, cleanup, "umount() failed");
-			mount_flag = 0;
-		}
-	}
+	{SYMLINK, UMOUNT_NOFOLLOW, EINVAL, 0,
+		"umount2('symlink', UMOUNT_NOFOLLOW) expected EINVAL"},
 
-	cleanup();
-	tst_exit();
-}
+	{MNTPOINT, UMOUNT_NOFOLLOW, 0, 0,
+		"umount2('mntpoint', UMOUNT_NOFOLLOW) expected success"},
+};
 
-static void setup(void)
+static void test_umount2(unsigned int n)
 {
-	tst_require_root();
-
-	if ((tst_kvercmp(2, 6, 8)) < 0) {
-		tst_brkm(TCONF, NULL, "This test can only run on kernels "
-			 "that are 2.6.8 or higher");
-	}
+	struct tcase *tc = &tcases[n];
 
-	tst_sig(NOFORK, DEF_HANDLER, NULL);
-
-	tst_tmpdir();
-
-	fs_type = tst_dev_fs_type();
-	device = tst_acquire_device(cleanup);
-
-	if (!device)
-		tst_brkm(TCONF, cleanup, "Failed to obtain block device");
-
-	tst_mkfs(cleanup, device, fs_type, NULL, NULL);
-
-	SAFE_MKDIR(cleanup, MNTPOINT, DIR_MODE);
-
-	TEST_PAUSE;
-}
-
-static void test_umount2(int i)
-{
-	/* a new access removes the expired mark of the mount point */
-	if (test_cases[i].do_access) {
-		if (access(MNTPOINT, F_OK) == -1)
-			tst_brkm(TBROK | TERRNO, cleanup, "access(2) failed");
+	if (!mount_flag) {
+		SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, NULL);
+		mount_flag = 1;
 	}
 
-	TEST(umount2_retry(MNTPOINT, test_cases[i].flag));
-
-	if (test_cases[i].exp_errno != 0)
-		verify_failure(i);
-	else
-		verify_success(i);
-}
+	tst_res(TINFO, "Testing %s", tc->desc);
 
-static void verify_failure(int i)
-{
-	if (TEST_RETURN == 0) {
-		tst_resm(TFAIL, "%s passed unexpectedly", test_cases[i].desc);
-		mount_flag = 0;
-		return;
-	}
+	if (tc->do_access)
+		SAFE_ACCESS(MNTPOINT, F_OK);
 
-	if (TEST_ERRNO != test_cases[i].exp_errno) {
-		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
-			 test_cases[i].desc);
-		return;
+	if (tc->exp_errno) {
+		TST_EXP_FAIL(umount2_retry(tc->mntpoint, tc->flag), tc->exp_errno,
+			"umount2_retry(%s, %d)", tc->mntpoint, tc->flag);
+		if (!TST_PASS)
+			mount_flag = 0;
+	} else {
+		TST_EXP_PASS(umount2_retry(tc->mntpoint, tc->flag),
+			"umount2_retry(%s, %d)", tc->mntpoint, tc->flag);
+		if (TST_PASS)
+			mount_flag = 0;
 	}
-
-	tst_resm(TPASS | TTERRNO, "umount2(2) failed as expected");
 }
 
-static void verify_success(int i)
+static void setup(void)
 {
-	if (TEST_RETURN != 0) {
-		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
-			 test_cases[i].desc);
-		return;
-	}
-
-	tst_resm(TPASS, "umount2(2) succeeded as expected");
-	mount_flag = 0;
+	SAFE_SYMLINK(MNTPOINT, SYMLINK);
 }
 
 static void cleanup(void)
 {
-	if (mount_flag && tst_umount(MNTPOINT))
-		tst_resm(TWARN | TERRNO, "Failed to unmount");
-
-	if (device)
-		tst_release_device(device);
-
-	tst_rmdir();
+	if (mount_flag)
+		SAFE_UMOUNT(MNTPOINT);
 }
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.cleanup = cleanup,
+	.setup = setup,
+	.needs_root = 1,
+	.needs_device = 1,
+	.format_device = 1,
+	.mntpoint = MNTPOINT,
+	.test = test_umount2,
+};
diff --git a/testcases/kernel/syscalls/umount2/umount2_03.c b/testcases/kernel/syscalls/umount2/umount2_03.c
deleted file mode 100644
index a8fddf6..0000000
--- a/testcases/kernel/syscalls/umount2/umount2_03.c
+++ /dev/null
@@ -1,167 +0,0 @@
-/*
- * Copyright (c) 2015 Fujitsu Ltd.
- * Author: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- *
- * You should have received a copy of the GNU General Public License
- * alone with this program.
- */
-
-/*
- * DESCRIPTION
- *  Test for feature UMOUNT_NOFOLLOW of umount2().
- *  "Don't dereference target if it is a symbolic link,
- *   and fails with the error EINVAL."
- */
-
-#include <errno.h>
-#include <sys/mount.h>
-
-#include "test.h"
-#include "safe_macros.h"
-#include "lapi/mount.h"
-
-#include "umount2.h"
-
-#define DIR_MODE	(S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH)
-#define MNTPOINT	"mntpoint"
-#define SYMLINK	"symlink"
-
-static void setup(void);
-static void test_umount2(int i);
-static void verify_failure(int i);
-static void verify_success(int i);
-static void cleanup(void);
-
-static const char *device;
-static const char *fs_type;
-
-static int mount_flag;
-
-static struct test_case_t {
-	const char *mntpoint;
-	int exp_errno;
-	const char *desc;
-} test_cases[] = {
-	{SYMLINK, EINVAL,
-		"umount2('symlink', UMOUNT_NOFOLLOW) expected EINVAL"},
-	{MNTPOINT, 0,
-		"umount2('mntpoint', UMOUNT_NOFOLLOW) expected success"},
-};
-
-char *TCID = "umount2_03";
-int TST_TOTAL = ARRAY_SIZE(test_cases);
-
-int main(int ac, char **av)
-{
-	int lc;
-	int tc;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-
-		for (tc = 0; tc < TST_TOTAL; tc++)
-			test_umount2(tc);
-	}
-
-	cleanup();
-	tst_exit();
-}
-
-static void setup(void)
-{
-	tst_require_root();
-
-	if ((tst_kvercmp(2, 6, 34)) < 0) {
-		tst_brkm(TCONF, NULL, "This test can only run on kernels "
-			 "that are 2.6.34 or higher");
-	}
-
-	tst_sig(NOFORK, DEF_HANDLER, NULL);
-
-	tst_tmpdir();
-
-	fs_type = tst_dev_fs_type();
-	device = tst_acquire_device(cleanup);
-
-	if (!device)
-		tst_brkm(TCONF, cleanup, "Failed to obtain block device");
-
-	tst_mkfs(cleanup, device, fs_type, NULL, NULL);
-
-	SAFE_MKDIR(cleanup, MNTPOINT, DIR_MODE);
-
-	SAFE_SYMLINK(cleanup, MNTPOINT, SYMLINK);
-
-	TEST_PAUSE;
-}
-
-static void test_umount2(int i)
-{
-	SAFE_MOUNT(cleanup, device, MNTPOINT, fs_type, 0, NULL);
-	mount_flag = 1;
-
-	TEST(umount2_retry(test_cases[i].mntpoint, UMOUNT_NOFOLLOW));
-
-	if (test_cases[i].exp_errno != 0)
-		verify_failure(i);
-	else
-		verify_success(i);
-
-	if (mount_flag) {
-		if (tst_umount(MNTPOINT))
-			tst_brkm(TBROK, cleanup, "umount() failed");
-		mount_flag = 0;
-	}
-}
-
-static void verify_failure(int i)
-{
-	if (TEST_RETURN == 0) {
-		tst_resm(TFAIL, "%s passed unexpectedly", test_cases[i].desc);
-		mount_flag = 0;
-		return;
-	}
-
-	if (TEST_ERRNO != test_cases[i].exp_errno) {
-		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
-			 test_cases[i].desc);
-		return;
-	}
-
-	tst_resm(TPASS | TTERRNO, "umount2(2) failed as expected");
-}
-
-static void verify_success(int i)
-{
-	if (TEST_RETURN != 0) {
-		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
-			 test_cases[i].desc);
-		return;
-	}
-
-	tst_resm(TPASS, "umount2(2) succeeded as expected");
-	mount_flag = 0;
-}
-
-static void cleanup(void)
-{
-	if (mount_flag && tst_umount(MNTPOINT))
-		tst_resm(TWARN | TERRNO, "Failed to unmount");
-
-	if (device)
-		tst_release_device(device);
-
-	tst_rmdir();
-}
-- 
1.8.3.1


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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [LTP] [PATCH v3 1/3] Add SAFE_ACCESS macro
  2022-03-18 18:55             ` [LTP] [PATCH v3 1/3] Add SAFE_ACCESS macro Dai Shili
  2022-03-18 18:55               ` [LTP] [PATCH v3 2/3] Make use of SAFE_ACCESS Dai Shili
  2022-03-18 18:55               ` [LTP] [PATCH v3 3/3] syscalls/umount2: Convert to new API and use SAFE_ACCESS Dai Shili
@ 2022-03-23 19:24               ` Petr Vorel
  2022-03-24  2:22                 ` daisl.fnst
  2022-03-23 19:27               ` Petr Vorel
  2022-03-23 19:33               ` Petr Vorel
  4 siblings, 1 reply; 27+ messages in thread
From: Petr Vorel @ 2022-03-23 19:24 UTC (permalink / raw)
  To: Dai Shili; +Cc: ltp

Hi Dai,

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Also adding Xu tag from previous version (Dai you can dare to add it for next
version if the patch does not change):
https://lore.kernel.org/ltp/62340304.1000500@fujitsu.com/
https://patchwork.ozlabs.org/project/ltp/patch/1647550813-1958-1-git-send-email-daisl.fnst@fujitsu.com/

Reviewed-by: Yang Xu <xuyang2018.jy@fujitsu.com>

Kind regards,
Petr

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [LTP] [PATCH v3 1/3] Add SAFE_ACCESS macro
  2022-03-18 18:55             ` [LTP] [PATCH v3 1/3] Add SAFE_ACCESS macro Dai Shili
                                 ` (2 preceding siblings ...)
  2022-03-23 19:24               ` [LTP] [PATCH v3 1/3] Add SAFE_ACCESS macro Petr Vorel
@ 2022-03-23 19:27               ` Petr Vorel
  2022-03-23 19:33               ` Petr Vorel
  4 siblings, 0 replies; 27+ messages in thread
From: Petr Vorel @ 2022-03-23 19:27 UTC (permalink / raw)
  To: Dai Shili; +Cc: ltp

Hi Dai,

also +1 for fixing whitespace in your changes :).

Kind regards,
Petr

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [LTP] [PATCH v3 2/3] Make use of SAFE_ACCESS
  2022-03-18 18:55               ` [LTP] [PATCH v3 2/3] Make use of SAFE_ACCESS Dai Shili
@ 2022-03-23 19:30                 ` Petr Vorel
  2022-03-23 19:33                 ` Petr Vorel
  1 sibling, 0 replies; 27+ messages in thread
From: Petr Vorel @ 2022-03-23 19:30 UTC (permalink / raw)
  To: Dai Shili; +Cc: ltp

Hi Dai,

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Also adding Xu's tag from v2:
https://patchwork.ozlabs.org/project/ltp/patch/1647550813-1958-2-git-send-email-daisl.fnst@fujitsu.com/

Reviewed-by: Yang Xu <xuyang2018.jy@fujitsu.com>

Kind regards,
Petr

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [LTP] [PATCH v3 1/3] Add SAFE_ACCESS macro
  2022-03-18 18:55             ` [LTP] [PATCH v3 1/3] Add SAFE_ACCESS macro Dai Shili
                                 ` (3 preceding siblings ...)
  2022-03-23 19:27               ` Petr Vorel
@ 2022-03-23 19:33               ` Petr Vorel
  4 siblings, 0 replies; 27+ messages in thread
From: Petr Vorel @ 2022-03-23 19:33 UTC (permalink / raw)
  To: Dai Shili; +Cc: ltp

Hi Dai, Xu,

merged this one.

Kind regards,
Petr

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [LTP] [PATCH v3 2/3] Make use of SAFE_ACCESS
  2022-03-18 18:55               ` [LTP] [PATCH v3 2/3] Make use of SAFE_ACCESS Dai Shili
  2022-03-23 19:30                 ` Petr Vorel
@ 2022-03-23 19:33                 ` Petr Vorel
  1 sibling, 0 replies; 27+ messages in thread
From: Petr Vorel @ 2022-03-23 19:33 UTC (permalink / raw)
  To: Dai Shili; +Cc: ltp

And also this one merged.

Kind regards,
Petr

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [LTP] [PATCH v3 3/3] syscalls/umount2: Convert to new API and use SAFE_ACCESS
  2022-03-18 18:55               ` [LTP] [PATCH v3 3/3] syscalls/umount2: Convert to new API and use SAFE_ACCESS Dai Shili
@ 2022-03-23 19:36                 ` Petr Vorel
  2022-03-24  2:23                   ` daisl.fnst
  2022-03-24 15:35                   ` [LTP] [PATCH v4] " Dai Shili
  2022-03-24  1:37                 ` [LTP] [PATCH v3 3/3] " xuyang2018.jy
  1 sibling, 2 replies; 27+ messages in thread
From: Petr Vorel @ 2022-03-23 19:36 UTC (permalink / raw)
  To: Dai Shili; +Cc: ltp

Hi Dai,

> +++ b/testcases/kernel/syscalls/umount2/umount2.h
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
>  /*
>   * Copyright (c) 2016 Cyril Hrubis <chrubis@suse.cz>
>   *
nit: if you add SPDX (good to do) you're supposed to remove GPL text
(i.e. remove: "This program is free software: you ...")

Can be done before merge.

Kind regards,
Petr

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [LTP] [PATCH v3 3/3] syscalls/umount2: Convert to new API and use SAFE_ACCESS
  2022-03-18 18:55               ` [LTP] [PATCH v3 3/3] syscalls/umount2: Convert to new API and use SAFE_ACCESS Dai Shili
  2022-03-23 19:36                 ` Petr Vorel
@ 2022-03-24  1:37                 ` xuyang2018.jy
  1 sibling, 0 replies; 27+ messages in thread
From: xuyang2018.jy @ 2022-03-24  1:37 UTC (permalink / raw)
  To: daisl.fnst; +Cc: ltp

Hi Dai

It seems you don't fix some problems thant I comment on your v2 patch.
so again

>   /*

Typo  /* =>/*\

So the description can be used in docparse.

ps:you can cd ltp/docparse directory, then run make command, it will
generate metadata.html, then put this html file into your nginx or
appache directory, then you can see this html whether works well for
docparse from other host.
> - * DESCRIPTION
> + * [Description]
> + *
>    *  Test for feature MNT_EXPIRE of umount2().
> - *  "Mark the mount point as expired.If a mount point is not currently
> - *   in use, then an initial call to umount2() with this flag fails with
> - *   the error EAGAIN, but marks the mount point as expired. The mount
> - *   point remains expired as long as it isn't accessed by any process.
> - *   A second umount2() call specifying MNT_EXPIRE unmounts an expired
> - *   mount point. This flag cannot be specified with either MNT_FORCE or
> - *   MNT_DETACH. (fails with the error EINVAL)"
> + *
> + * - EINVAL when flag is specified with either MNT_FORCE or MNT_DETACH
> + * - EAGAIN when initial call to umount2(2) with MNT_EXPIRE
> + * - EAGAIN when umount2(2) with MNT_EXPIRE after access(2)
> + * - succeed when second call to umount2(2) with MNT_EXPIRE
> + *
> + *  Test for feature UMOUNT_NOFOLLOW of umount2().
> + *
> + * - EINVAL when target is a symbolic link
> + * - succeed when target is a mount point
>    */
> 
> -#include<errno.h>
>   #include<sys/mount.h>
> -
> -#include "test.h"
> -#include "safe_macros.h"
>   #include "lapi/mount.h"
> -
> +#include "tst_test.h"
>   #include "umount2.h"
> 
> -#define DIR_MODE	(S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH)
> -#define MNTPOINT	"mntpoint"
> -
> -static void setup(void);
> -static void test_umount2(int i);
> -static void verify_failure(int i);
> -static void verify_success(int i);
> -static void cleanup(void);
> -
> -static const char *device;
> -static const char *fs_type;
> +#define MNTPOINT        "mntpoint"
> +#define SYMLINK	"symlink"
> 
>   static int mount_flag;
> 
> -static struct test_case_t {
> +static struct tcase {
> +	const char *mntpoint;
>   	int flag;
>   	int exp_errno;
>   	int do_access;
>   	const char *desc;
> -} test_cases[] = {
> -	{MNT_EXPIRE | MNT_FORCE, EINVAL, 0,
> -		"umount2(2) with MNT_EXPIRE | MNT_FORCE expected EINVAL"},
> -	{MNT_EXPIRE | MNT_DETACH, EINVAL, 0,
> -		"umount2(2) with MNT_EXPIRE | MNT_DETACH expected EINVAL"},
> -	{MNT_EXPIRE, EAGAIN, 0,
> -		"initial call to umount2(2) with MNT_EXPIRE expected EAGAIN"},
> -	{MNT_EXPIRE, EAGAIN, 1,
> -		"umount2(2) with MNT_EXPIRE after access(2) expected EAGAIN"},
> -	{MNT_EXPIRE, 0, 0,
> -		"second call to umount2(2) with MNT_EXPIRE expected success"},
> -};
> +} tcases[] = {
> +	{MNTPOINT, MNT_EXPIRE | MNT_FORCE, EINVAL, 0,
> +		"umount2() with MNT_EXPIRE | MNT_FORCE expected EINVAL"},
> 
> -char *TCID = "umount2_02";
> -int TST_TOTAL = ARRAY_SIZE(test_cases);
> +	{MNTPOINT, MNT_EXPIRE | MNT_DETACH, EINVAL, 0,
> +		"umount2() with MNT_EXPIRE | MNT_DETACH expected EINVAL"},
> 
> -int main(int ac, char **av)
> -{
> -	int lc;
> -	int tc;
> -
> -	tst_parse_opts(ac, av, NULL, NULL);
> -
> -	setup();
> +	{MNTPOINT, MNT_EXPIRE, EAGAIN, 0,
> +		"initial call to umount2() with MNT_EXPIRE expected EAGAIN"},
> 
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -		tst_count = 0;
> +	{MNTPOINT, MNT_EXPIRE, EAGAIN, 1,
> +		"umount2() with MNT_EXPIRE after access() expected EAGAIN"},
> 
> -		SAFE_MOUNT(cleanup, device, MNTPOINT, fs_type, 0, NULL);
> -		mount_flag = 1;
> -
> -		for (tc = 0; tc<  TST_TOTAL; tc++)
> -			test_umount2(tc);
> +	{MNTPOINT, MNT_EXPIRE, 0, 0,
> +		"second call to umount2() with MNT_EXPIRE expected success"},
> 
> -		if (mount_flag) {
> -			if (tst_umount(MNTPOINT))
> -				tst_brkm(TBROK, cleanup, "umount() failed");
> -			mount_flag = 0;
> -		}
> -	}
> +	{SYMLINK, UMOUNT_NOFOLLOW, EINVAL, 0,
> +		"umount2('symlink', UMOUNT_NOFOLLOW) expected EINVAL"},
> 
> -	cleanup();
> -	tst_exit();
> -}
> +	{MNTPOINT, UMOUNT_NOFOLLOW, 0, 0,
> +		"umount2('mntpoint', UMOUNT_NOFOLLOW) expected success"},
> +};
> 
> -static void setup(void)
> +static void test_umount2(unsigned int n)
>   {
> -	tst_require_root();
> -
> -	if ((tst_kvercmp(2, 6, 8))<  0) {
> -		tst_brkm(TCONF, NULL, "This test can only run on kernels "
> -			 "that are 2.6.8 or higher");
> -	}
> +	struct tcase *tc =&tcases[n];
> 
> -	tst_sig(NOFORK, DEF_HANDLER, NULL);
> -
> -	tst_tmpdir();
> -
> -	fs_type = tst_dev_fs_type();
> -	device = tst_acquire_device(cleanup);
> -
> -	if (!device)
> -		tst_brkm(TCONF, cleanup, "Failed to obtain block device");
> -
> -	tst_mkfs(cleanup, device, fs_type, NULL, NULL);
> -
> -	SAFE_MKDIR(cleanup, MNTPOINT, DIR_MODE);
> -
> -	TEST_PAUSE;
> -}
> -
> -static void test_umount2(int i)
> -{
> -	/* a new access removes the expired mark of the mount point */
> -	if (test_cases[i].do_access) {
> -		if (access(MNTPOINT, F_OK) == -1)
> -			tst_brkm(TBROK | TERRNO, cleanup, "access(2) failed");
> +	if (!mount_flag) {
> +		SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, NULL);
> +		mount_flag = 1;
>   	}
> 
> -	TEST(umount2_retry(MNTPOINT, test_cases[i].flag));
> -
> -	if (test_cases[i].exp_errno != 0)
> -		verify_failure(i);
> -	else
> -		verify_success(i);
> -}
> +	tst_res(TINFO, "Testing %s", tc->desc);
> 
> -static void verify_failure(int i)
> -{
> -	if (TEST_RETURN == 0) {
> -		tst_resm(TFAIL, "%s passed unexpectedly", test_cases[i].desc);
> -		mount_flag = 0;
> -		return;
> -	}
> +	if (tc->do_access)
> +		SAFE_ACCESS(MNTPOINT, F_OK);
> 
> -	if (TEST_ERRNO != test_cases[i].exp_errno) {
> -		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
> -			 test_cases[i].desc);
> -		return;
> +	if (tc->exp_errno) {
> +		TST_EXP_FAIL(umount2_retry(tc->mntpoint, tc->flag), tc->exp_errno,
> +			"umount2_retry(%s, %d)", tc->mntpoint, tc->flag);
> +		if (!TST_PASS)
> +			mount_flag = 0;
> +	} else {
> +		TST_EXP_PASS(umount2_retry(tc->mntpoint, tc->flag),
> +			"umount2_retry(%s, %d)", tc->mntpoint, tc->flag);
> +		if (TST_PASS)
> +			mount_flag = 0;
>   	}
> -
> -	tst_resm(TPASS | TTERRNO, "umount2(2) failed as expected");
>   }
> 
> -static void verify_success(int i)
> +static void setup(void)
>   {
> -	if (TEST_RETURN != 0) {
> -		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
> -			 test_cases[i].desc);
> -		return;
> -	}
> -
> -	tst_resm(TPASS, "umount2(2) succeeded as expected");
> -	mount_flag = 0;
> +	SAFE_SYMLINK(MNTPOINT, SYMLINK);
>   }
> 
>   static void cleanup(void)
>   {
> -	if (mount_flag&&  tst_umount(MNTPOINT))
> -		tst_resm(TWARN | TERRNO, "Failed to unmount");
> -
> -	if (device)
> -		tst_release_device(device);
> -
> -	tst_rmdir();
> +	if (mount_flag)
> +		SAFE_UMOUNT(MNTPOINT);
>   }
> +
> +static struct tst_test test = {
> +	.tcnt = ARRAY_SIZE(tcases),
> +	.cleanup = cleanup,
> +	.setup = setup,
> +	.needs_root = 1,
> +	.needs_device = 1,
> +	.format_device = 1,
If we specify format_device, we don't need .needs_device because
format_device will enable needs_device automatically.

Best Regards
Yang Xu
> +	.mntpoint = MNTPOINT,
> +	.test = test_umount2,
> +};

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [LTP] [PATCH v3 1/3] Add SAFE_ACCESS macro
  2022-03-23 19:24               ` [LTP] [PATCH v3 1/3] Add SAFE_ACCESS macro Petr Vorel
@ 2022-03-24  2:22                 ` daisl.fnst
  0 siblings, 0 replies; 27+ messages in thread
From: daisl.fnst @ 2022-03-24  2:22 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

Hi Petr,

在 2022/3/24 3:24, Petr Vorel 写道:
> Hi Dai,
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> Also adding Xu tag from previous version (Dai you can dare to add it for next
> version if the patch does not change):
> https://lore.kernel.org/ltp/62340304.1000500@fujitsu.com/
> https://patchwork.ozlabs.org/project/ltp/patch/1647550813-1958-1-git-send-email-daisl.fnst@fujitsu.com/
>
> Reviewed-by: Yang Xu <xuyang2018.jy@fujitsu.com>
Got it. I'll pay attention next time.
> Kind regards,
> Petr

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [LTP] [PATCH v3 3/3] syscalls/umount2: Convert to new API and use SAFE_ACCESS
  2022-03-23 19:36                 ` Petr Vorel
@ 2022-03-24  2:23                   ` daisl.fnst
  2022-03-24 15:35                   ` [LTP] [PATCH v4] " Dai Shili
  1 sibling, 0 replies; 27+ messages in thread
From: daisl.fnst @ 2022-03-24  2:23 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

Hi Petr,

在 2022/3/24 3:36, Petr Vorel 写道:
> Hi Dai,
>
>> +++ b/testcases/kernel/syscalls/umount2/umount2.h
>> @@ -1,3 +1,4 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>   /*
>>    * Copyright (c) 2016 Cyril Hrubis <chrubis@suse.cz>
>>    *
> nit: if you add SPDX (good to do) you're supposed to remove GPL text
> (i.e. remove: "This program is free software: you ...")
>
> Can be done before merge.
Got it. I will send v4.
> Kind regards,
> Petr

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [LTP] [PATCH v4] syscalls/umount2: Convert to new API and use SAFE_ACCESS
  2022-03-24 15:35                   ` [LTP] [PATCH v4] " Dai Shili
@ 2022-03-24  9:11                     ` Petr Vorel
  2022-03-25  3:00                       ` daisl.fnst
  2022-03-25 17:29                       ` [LTP] [PATCH v5] " Dai Shili
  0 siblings, 2 replies; 27+ messages in thread
From: Petr Vorel @ 2022-03-24  9:11 UTC (permalink / raw)
  To: Dai Shili; +Cc: ltp

Hi Dai,

We're nearly there, IMHO 2 minor changes are required.

nit: you can help reviewers to list changelog from previous version (for your
next patches), see:

https://patchwork.ozlabs.org/project/ltp/patch/20220324022455.245300-1-zhaogongyi@huawei.com/
https://patchwork.ozlabs.org/project/ltp/cover/20211103120233.20728-1-chrubis@suse.cz/

> 1. use TST_EXP_FAIL and TST_EXP_PASS macro
> 2. use SAFE macro
> 3. simplify verify operations
> 4. merge umount2_03 to umount2_02

very nit: more important that "use SAFE macro" (that's one of the main reasons
why we use new API) is to mention the reason why to merge tests.

...
> +/*\
> + * [Description]
> + *
>   *  Test for feature MNT_EXPIRE of umount2().
This must be:
  * Test for feature MNT_EXPIRE of umount2().
extra space after '* ' leads text to be formatted as <pre> or <code>.

> - *  "Mark the mount point as expired.If a mount point is not currently
> - *   in use, then an initial call to umount2() with this flag fails with
> - *   the error EAGAIN, but marks the mount point as expired. The mount
> - *   point remains expired as long as it isn't accessed by any process.
> - *   A second umount2() call specifying MNT_EXPIRE unmounts an expired
> - *   mount point. This flag cannot be specified with either MNT_FORCE or
> - *   MNT_DETACH. (fails with the error EINVAL)"
> + *
> + * - EINVAL when flag is specified with either MNT_FORCE or MNT_DETACH
> + * - EAGAIN when initial call to umount2(2) with MNT_EXPIRE
> + * - EAGAIN when umount2(2) with MNT_EXPIRE after access(2)
> + * - succeed when second call to umount2(2) with MNT_EXPIRE
> + *
> + *  Test for feature UMOUNT_NOFOLLOW of umount2().
And here as well:
* Test for feature UMOUNT_NOFOLLOW of umount2().

run make in metadata/ and then check docparse/metadata.html
(or docparse/metadata.pdf).

(1st thing to change)

> + *
> + * - EINVAL when target is a symbolic link
> + * - succeed when target is a mount point
>   */

> -#include <errno.h>
>  #include <sys/mount.h>
> -
> -#include "test.h"
> -#include "safe_macros.h"
>  #include "lapi/mount.h"
> -
> +#include "tst_test.h"
>  #include "umount2.h"
umount2.h is now used only in this test. Please put the content into the test
and delete the file (2nd change to do).

...
> +} tcases[] = {
> +	{MNTPOINT, MNT_EXPIRE | MNT_FORCE, EINVAL, 0,
> +		"umount2() with MNT_EXPIRE | MNT_FORCE expected EINVAL"},
nit: I was thinking about using macro to not repeat to flag and exp_errno,
(something similar to POLICY_DESC_TEXT(x, y) in
testcases/kernel/syscalls/mbind/mbind01.c), but as description varies a lot it's
probably better to keep it as is (unless you rephrase it).

...

> +	{SYMLINK, UMOUNT_NOFOLLOW, EINVAL, 0,
> +		"umount2('symlink', UMOUNT_NOFOLLOW) expected EINVAL"},
...
> +	{MNTPOINT, UMOUNT_NOFOLLOW, 0, 0,
> +		"umount2('mntpoint', UMOUNT_NOFOLLOW) expected success"},
> +};

...
> +	if (tc->do_access)
> +		SAFE_ACCESS(MNTPOINT, F_OK);

> -	if (TEST_ERRNO != test_cases[i].exp_errno) {
> -		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
> -			 test_cases[i].desc);
> -		return;
> +	if (tc->exp_errno) {
> +		TST_EXP_FAIL(umount2_retry(tc->mntpoint, tc->flag), tc->exp_errno,
> +			"umount2_retry(%s, %d)", tc->mntpoint, tc->flag);
> +		if (!TST_PASS)
> +			mount_flag = 0;
> +	} else {
> +		TST_EXP_PASS(umount2_retry(tc->mntpoint, tc->flag),
> +			"umount2_retry(%s, %d)", tc->mntpoint, tc->flag);
> +		if (TST_PASS)
> +			mount_flag = 0;
>  	}

nit: this would be more compact:

	if (tc->exp_errno)
		TST_EXP_FAIL(umount2_retry(tc->mntpoint, tc->flag), tc->exp_errno,
			"umount2_retry(%s, %d)", tc->mntpoint, tc->flag);
	else
		TST_EXP_PASS(umount2_retry(tc->mntpoint, tc->flag),
			"umount2_retry(%s, %d)", tc->mntpoint, tc->flag);

	if (!!tc->exp_errno ^ !!TST_PASS)
		mount_flag = 0;

Kind regards,
Petr

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [LTP] [PATCH v4] syscalls/umount2: Convert to new API and use SAFE_ACCESS
  2022-03-23 19:36                 ` Petr Vorel
  2022-03-24  2:23                   ` daisl.fnst
@ 2022-03-24 15:35                   ` Dai Shili
  2022-03-24  9:11                     ` Petr Vorel
  1 sibling, 1 reply; 27+ messages in thread
From: Dai Shili @ 2022-03-24 15:35 UTC (permalink / raw)
  To: pvorel; +Cc: ltp

1. use TST_EXP_FAIL and TST_EXP_PASS macro
2. use SAFE macro
3. simplify verify operations
4. merge umount2_03 to umount2_02

Signed-off-by: Dai Shili <daisl.fnst@fujitsu.com>
---
 runtest/syscalls                               |   1 -
 testcases/kernel/syscalls/umount2/.gitignore   |   1 -
 testcases/kernel/syscalls/umount2/umount2.h    |  18 +--
 testcases/kernel/syscalls/umount2/umount2_02.c | 210 ++++++++-----------------
 testcases/kernel/syscalls/umount2/umount2_03.c | 167 --------------------
 5 files changed, 71 insertions(+), 326 deletions(-)
 delete mode 100644 testcases/kernel/syscalls/umount2/umount2_03.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 6186bfc..0eea946 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1647,7 +1647,6 @@ umount03 umount03
 
 umount2_01 umount2_01
 umount2_02 umount2_02
-umount2_03 umount2_03
 
 userfaultfd01 userfaultfd01
 
diff --git a/testcases/kernel/syscalls/umount2/.gitignore b/testcases/kernel/syscalls/umount2/.gitignore
index 1d6a0e8..e2056bf 100644
--- a/testcases/kernel/syscalls/umount2/.gitignore
+++ b/testcases/kernel/syscalls/umount2/.gitignore
@@ -1,3 +1,2 @@
 /umount2_01
 /umount2_02
-/umount2_03
diff --git a/testcases/kernel/syscalls/umount2/umount2.h b/testcases/kernel/syscalls/umount2/umount2.h
index 65e4c24..22df8c6 100644
--- a/testcases/kernel/syscalls/umount2/umount2.h
+++ b/testcases/kernel/syscalls/umount2/umount2.h
@@ -1,18 +1,6 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) 2016 Cyril Hrubis <chrubis@suse.cz>
- *
- * 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, see <http://www.gnu.org/licenses/>.
  */
 
 #ifndef UMOUNT2_H__
@@ -28,13 +16,13 @@ static inline int umount2_retry(const char *target, int flags)
 		if (ret == 0 || errno != EBUSY)
 			return ret;
 
-		tst_resm(TINFO, "umount('%s', %i) failed with EBUSY, try %2i...",
+		tst_res(TINFO, "umount('%s', %i) failed with EBUSY, try %2i...",
 			 target, flags, i);
 
 		usleep(100000);
 	}
 
-	tst_resm(TWARN, "Failed to umount('%s', %i) after 50 retries",
+	tst_res(TWARN, "Failed to umount('%s', %i) after 50 retries",
 	         target, flags);
 
 	errno = EBUSY;
diff --git a/testcases/kernel/syscalls/umount2/umount2_02.c b/testcases/kernel/syscalls/umount2/umount2_02.c
index 7d558fa..8d893c0 100644
--- a/testcases/kernel/syscalls/umount2/umount2_02.c
+++ b/testcases/kernel/syscalls/umount2/umount2_02.c
@@ -1,182 +1,108 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * Copyright (c) 2015 Fujitsu Ltd.
+ * Copyright (c) 2015-2022 FUJITSU LIMITED. All rights reserved
  * Author: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- *
- * You should have received a copy of the GNU General Public License
- * alone with this program.
  */
 
-/*
- * DESCRIPTION
+/*\
+ * [Description]
+ *
  *  Test for feature MNT_EXPIRE of umount2().
- *  "Mark the mount point as expired.If a mount point is not currently
- *   in use, then an initial call to umount2() with this flag fails with
- *   the error EAGAIN, but marks the mount point as expired. The mount
- *   point remains expired as long as it isn't accessed by any process.
- *   A second umount2() call specifying MNT_EXPIRE unmounts an expired
- *   mount point. This flag cannot be specified with either MNT_FORCE or
- *   MNT_DETACH. (fails with the error EINVAL)"
+ *
+ * - EINVAL when flag is specified with either MNT_FORCE or MNT_DETACH
+ * - EAGAIN when initial call to umount2(2) with MNT_EXPIRE
+ * - EAGAIN when umount2(2) with MNT_EXPIRE after access(2)
+ * - succeed when second call to umount2(2) with MNT_EXPIRE
+ *
+ *  Test for feature UMOUNT_NOFOLLOW of umount2().
+ *
+ * - EINVAL when target is a symbolic link
+ * - succeed when target is a mount point
  */
 
-#include <errno.h>
 #include <sys/mount.h>
-
-#include "test.h"
-#include "safe_macros.h"
 #include "lapi/mount.h"
-
+#include "tst_test.h"
 #include "umount2.h"
 
-#define DIR_MODE	(S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH)
-#define MNTPOINT	"mntpoint"
-
-static void setup(void);
-static void test_umount2(int i);
-static void verify_failure(int i);
-static void verify_success(int i);
-static void cleanup(void);
-
-static const char *device;
-static const char *fs_type;
+#define MNTPOINT        "mntpoint"
+#define SYMLINK	"symlink"
 
 static int mount_flag;
 
-static struct test_case_t {
+static struct tcase {
+	const char *mntpoint;
 	int flag;
 	int exp_errno;
 	int do_access;
 	const char *desc;
-} test_cases[] = {
-	{MNT_EXPIRE | MNT_FORCE, EINVAL, 0,
-		"umount2(2) with MNT_EXPIRE | MNT_FORCE expected EINVAL"},
-	{MNT_EXPIRE | MNT_DETACH, EINVAL, 0,
-		"umount2(2) with MNT_EXPIRE | MNT_DETACH expected EINVAL"},
-	{MNT_EXPIRE, EAGAIN, 0,
-		"initial call to umount2(2) with MNT_EXPIRE expected EAGAIN"},
-	{MNT_EXPIRE, EAGAIN, 1,
-		"umount2(2) with MNT_EXPIRE after access(2) expected EAGAIN"},
-	{MNT_EXPIRE, 0, 0,
-		"second call to umount2(2) with MNT_EXPIRE expected success"},
-};
+} tcases[] = {
+	{MNTPOINT, MNT_EXPIRE | MNT_FORCE, EINVAL, 0,
+		"umount2() with MNT_EXPIRE | MNT_FORCE expected EINVAL"},
 
-char *TCID = "umount2_02";
-int TST_TOTAL = ARRAY_SIZE(test_cases);
+	{MNTPOINT, MNT_EXPIRE | MNT_DETACH, EINVAL, 0,
+		"umount2() with MNT_EXPIRE | MNT_DETACH expected EINVAL"},
 
-int main(int ac, char **av)
-{
-	int lc;
-	int tc;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
+	{MNTPOINT, MNT_EXPIRE, EAGAIN, 0,
+		"initial call to umount2() with MNT_EXPIRE expected EAGAIN"},
 
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
+	{MNTPOINT, MNT_EXPIRE, EAGAIN, 1,
+		"umount2() with MNT_EXPIRE after access() expected EAGAIN"},
 
-		SAFE_MOUNT(cleanup, device, MNTPOINT, fs_type, 0, NULL);
-		mount_flag = 1;
-
-		for (tc = 0; tc < TST_TOTAL; tc++)
-			test_umount2(tc);
+	{MNTPOINT, MNT_EXPIRE, 0, 0,
+		"second call to umount2() with MNT_EXPIRE expected success"},
 
-		if (mount_flag) {
-			if (tst_umount(MNTPOINT))
-				tst_brkm(TBROK, cleanup, "umount() failed");
-			mount_flag = 0;
-		}
-	}
+	{SYMLINK, UMOUNT_NOFOLLOW, EINVAL, 0,
+		"umount2('symlink', UMOUNT_NOFOLLOW) expected EINVAL"},
 
-	cleanup();
-	tst_exit();
-}
+	{MNTPOINT, UMOUNT_NOFOLLOW, 0, 0,
+		"umount2('mntpoint', UMOUNT_NOFOLLOW) expected success"},
+};
 
-static void setup(void)
+static void test_umount2(unsigned int n)
 {
-	tst_require_root();
-
-	if ((tst_kvercmp(2, 6, 8)) < 0) {
-		tst_brkm(TCONF, NULL, "This test can only run on kernels "
-			 "that are 2.6.8 or higher");
-	}
+	struct tcase *tc = &tcases[n];
 
-	tst_sig(NOFORK, DEF_HANDLER, NULL);
-
-	tst_tmpdir();
-
-	fs_type = tst_dev_fs_type();
-	device = tst_acquire_device(cleanup);
-
-	if (!device)
-		tst_brkm(TCONF, cleanup, "Failed to obtain block device");
-
-	tst_mkfs(cleanup, device, fs_type, NULL, NULL);
-
-	SAFE_MKDIR(cleanup, MNTPOINT, DIR_MODE);
-
-	TEST_PAUSE;
-}
-
-static void test_umount2(int i)
-{
-	/* a new access removes the expired mark of the mount point */
-	if (test_cases[i].do_access) {
-		if (access(MNTPOINT, F_OK) == -1)
-			tst_brkm(TBROK | TERRNO, cleanup, "access(2) failed");
+	if (!mount_flag) {
+		SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, NULL);
+		mount_flag = 1;
 	}
 
-	TEST(umount2_retry(MNTPOINT, test_cases[i].flag));
-
-	if (test_cases[i].exp_errno != 0)
-		verify_failure(i);
-	else
-		verify_success(i);
-}
+	tst_res(TINFO, "Testing %s", tc->desc);
 
-static void verify_failure(int i)
-{
-	if (TEST_RETURN == 0) {
-		tst_resm(TFAIL, "%s passed unexpectedly", test_cases[i].desc);
-		mount_flag = 0;
-		return;
-	}
+	if (tc->do_access)
+		SAFE_ACCESS(MNTPOINT, F_OK);
 
-	if (TEST_ERRNO != test_cases[i].exp_errno) {
-		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
-			 test_cases[i].desc);
-		return;
+	if (tc->exp_errno) {
+		TST_EXP_FAIL(umount2_retry(tc->mntpoint, tc->flag), tc->exp_errno,
+			"umount2_retry(%s, %d)", tc->mntpoint, tc->flag);
+		if (!TST_PASS)
+			mount_flag = 0;
+	} else {
+		TST_EXP_PASS(umount2_retry(tc->mntpoint, tc->flag),
+			"umount2_retry(%s, %d)", tc->mntpoint, tc->flag);
+		if (TST_PASS)
+			mount_flag = 0;
 	}
-
-	tst_resm(TPASS | TTERRNO, "umount2(2) failed as expected");
 }
 
-static void verify_success(int i)
+static void setup(void)
 {
-	if (TEST_RETURN != 0) {
-		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
-			 test_cases[i].desc);
-		return;
-	}
-
-	tst_resm(TPASS, "umount2(2) succeeded as expected");
-	mount_flag = 0;
+	SAFE_SYMLINK(MNTPOINT, SYMLINK);
 }
 
 static void cleanup(void)
 {
-	if (mount_flag && tst_umount(MNTPOINT))
-		tst_resm(TWARN | TERRNO, "Failed to unmount");
-
-	if (device)
-		tst_release_device(device);
-
-	tst_rmdir();
+	if (mount_flag)
+		SAFE_UMOUNT(MNTPOINT);
 }
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.cleanup = cleanup,
+	.setup = setup,
+	.needs_root = 1,
+	.format_device = 1,
+	.mntpoint = MNTPOINT,
+	.test = test_umount2,
+};
diff --git a/testcases/kernel/syscalls/umount2/umount2_03.c b/testcases/kernel/syscalls/umount2/umount2_03.c
deleted file mode 100644
index a8fddf6..0000000
--- a/testcases/kernel/syscalls/umount2/umount2_03.c
+++ /dev/null
@@ -1,167 +0,0 @@
-/*
- * Copyright (c) 2015 Fujitsu Ltd.
- * Author: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- *
- * You should have received a copy of the GNU General Public License
- * alone with this program.
- */
-
-/*
- * DESCRIPTION
- *  Test for feature UMOUNT_NOFOLLOW of umount2().
- *  "Don't dereference target if it is a symbolic link,
- *   and fails with the error EINVAL."
- */
-
-#include <errno.h>
-#include <sys/mount.h>
-
-#include "test.h"
-#include "safe_macros.h"
-#include "lapi/mount.h"
-
-#include "umount2.h"
-
-#define DIR_MODE	(S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH)
-#define MNTPOINT	"mntpoint"
-#define SYMLINK	"symlink"
-
-static void setup(void);
-static void test_umount2(int i);
-static void verify_failure(int i);
-static void verify_success(int i);
-static void cleanup(void);
-
-static const char *device;
-static const char *fs_type;
-
-static int mount_flag;
-
-static struct test_case_t {
-	const char *mntpoint;
-	int exp_errno;
-	const char *desc;
-} test_cases[] = {
-	{SYMLINK, EINVAL,
-		"umount2('symlink', UMOUNT_NOFOLLOW) expected EINVAL"},
-	{MNTPOINT, 0,
-		"umount2('mntpoint', UMOUNT_NOFOLLOW) expected success"},
-};
-
-char *TCID = "umount2_03";
-int TST_TOTAL = ARRAY_SIZE(test_cases);
-
-int main(int ac, char **av)
-{
-	int lc;
-	int tc;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-
-		for (tc = 0; tc < TST_TOTAL; tc++)
-			test_umount2(tc);
-	}
-
-	cleanup();
-	tst_exit();
-}
-
-static void setup(void)
-{
-	tst_require_root();
-
-	if ((tst_kvercmp(2, 6, 34)) < 0) {
-		tst_brkm(TCONF, NULL, "This test can only run on kernels "
-			 "that are 2.6.34 or higher");
-	}
-
-	tst_sig(NOFORK, DEF_HANDLER, NULL);
-
-	tst_tmpdir();
-
-	fs_type = tst_dev_fs_type();
-	device = tst_acquire_device(cleanup);
-
-	if (!device)
-		tst_brkm(TCONF, cleanup, "Failed to obtain block device");
-
-	tst_mkfs(cleanup, device, fs_type, NULL, NULL);
-
-	SAFE_MKDIR(cleanup, MNTPOINT, DIR_MODE);
-
-	SAFE_SYMLINK(cleanup, MNTPOINT, SYMLINK);
-
-	TEST_PAUSE;
-}
-
-static void test_umount2(int i)
-{
-	SAFE_MOUNT(cleanup, device, MNTPOINT, fs_type, 0, NULL);
-	mount_flag = 1;
-
-	TEST(umount2_retry(test_cases[i].mntpoint, UMOUNT_NOFOLLOW));
-
-	if (test_cases[i].exp_errno != 0)
-		verify_failure(i);
-	else
-		verify_success(i);
-
-	if (mount_flag) {
-		if (tst_umount(MNTPOINT))
-			tst_brkm(TBROK, cleanup, "umount() failed");
-		mount_flag = 0;
-	}
-}
-
-static void verify_failure(int i)
-{
-	if (TEST_RETURN == 0) {
-		tst_resm(TFAIL, "%s passed unexpectedly", test_cases[i].desc);
-		mount_flag = 0;
-		return;
-	}
-
-	if (TEST_ERRNO != test_cases[i].exp_errno) {
-		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
-			 test_cases[i].desc);
-		return;
-	}
-
-	tst_resm(TPASS | TTERRNO, "umount2(2) failed as expected");
-}
-
-static void verify_success(int i)
-{
-	if (TEST_RETURN != 0) {
-		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
-			 test_cases[i].desc);
-		return;
-	}
-
-	tst_resm(TPASS, "umount2(2) succeeded as expected");
-	mount_flag = 0;
-}
-
-static void cleanup(void)
-{
-	if (mount_flag && tst_umount(MNTPOINT))
-		tst_resm(TWARN | TERRNO, "Failed to unmount");
-
-	if (device)
-		tst_release_device(device);
-
-	tst_rmdir();
-}
-- 
1.8.3.1


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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [LTP] [PATCH v4] syscalls/umount2: Convert to new API and use SAFE_ACCESS
  2022-03-24  9:11                     ` Petr Vorel
@ 2022-03-25  3:00                       ` daisl.fnst
  2022-03-25 17:29                       ` [LTP] [PATCH v5] " Dai Shili
  1 sibling, 0 replies; 27+ messages in thread
From: daisl.fnst @ 2022-03-25  3:00 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

Hi Petr,

Thanks for review, I will send v5 later.

在 2022/3/24 17:11, Petr Vorel 写道:
> Hi Dai,
>
> We're nearly there, IMHO 2 minor changes are required.
>
> nit: you can help reviewers to list changelog from previous version (for your
> next patches), see:
>
> https://patchwork.ozlabs.org/project/ltp/patch/20220324022455.245300-1-zhaogongyi@huawei.com/
> https://patchwork.ozlabs.org/project/ltp/cover/20211103120233.20728-1-chrubis@suse.cz/
>
>> 1. use TST_EXP_FAIL and TST_EXP_PASS macro
>> 2. use SAFE macro
>> 3. simplify verify operations
>> 4. merge umount2_03 to umount2_02
> very nit: more important that "use SAFE macro" (that's one of the main reasons
> why we use new API) is to mention the reason why to merge tests.
>
> ...
>> +/*\
>> + * [Description]
>> + *
>>    *  Test for feature MNT_EXPIRE of umount2().
> This must be:
>    * Test for feature MNT_EXPIRE of umount2().
> extra space after '* ' leads text to be formatted as <pre> or <code>.
>
>> - *  "Mark the mount point as expired.If a mount point is not currently
>> - *   in use, then an initial call to umount2() with this flag fails with
>> - *   the error EAGAIN, but marks the mount point as expired. The mount
>> - *   point remains expired as long as it isn't accessed by any process.
>> - *   A second umount2() call specifying MNT_EXPIRE unmounts an expired
>> - *   mount point. This flag cannot be specified with either MNT_FORCE or
>> - *   MNT_DETACH. (fails with the error EINVAL)"
>> + *
>> + * - EINVAL when flag is specified with either MNT_FORCE or MNT_DETACH
>> + * - EAGAIN when initial call to umount2(2) with MNT_EXPIRE
>> + * - EAGAIN when umount2(2) with MNT_EXPIRE after access(2)
>> + * - succeed when second call to umount2(2) with MNT_EXPIRE
>> + *
>> + *  Test for feature UMOUNT_NOFOLLOW of umount2().
> And here as well:
> * Test for feature UMOUNT_NOFOLLOW of umount2().
>
> run make in metadata/ and then check docparse/metadata.html
> (or docparse/metadata.pdf).
>
> (1st thing to change)
>
>> + *
>> + * - EINVAL when target is a symbolic link
>> + * - succeed when target is a mount point
>>    */
>> -#include <errno.h>
>>   #include <sys/mount.h>
>> -
>> -#include "test.h"
>> -#include "safe_macros.h"
>>   #include "lapi/mount.h"
>> -
>> +#include "tst_test.h"
>>   #include "umount2.h"
> umount2.h is now used only in this test. Please put the content into the test
> and delete the file (2nd change to do).
>
> ...
>> +} tcases[] = {
>> +	{MNTPOINT, MNT_EXPIRE | MNT_FORCE, EINVAL, 0,
>> +		"umount2() with MNT_EXPIRE | MNT_FORCE expected EINVAL"},
> nit: I was thinking about using macro to not repeat to flag and exp_errno,
> (something similar to POLICY_DESC_TEXT(x, y) in
> testcases/kernel/syscalls/mbind/mbind01.c), but as description varies a lot it's
> probably better to keep it as is (unless you rephrase it).
>
> ...
>
>> +	{SYMLINK, UMOUNT_NOFOLLOW, EINVAL, 0,
>> +		"umount2('symlink', UMOUNT_NOFOLLOW) expected EINVAL"},
> ...
>> +	{MNTPOINT, UMOUNT_NOFOLLOW, 0, 0,
>> +		"umount2('mntpoint', UMOUNT_NOFOLLOW) expected success"},
>> +};
> ...
>> +	if (tc->do_access)
>> +		SAFE_ACCESS(MNTPOINT, F_OK);
>> -	if (TEST_ERRNO != test_cases[i].exp_errno) {
>> -		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
>> -			 test_cases[i].desc);
>> -		return;
>> +	if (tc->exp_errno) {
>> +		TST_EXP_FAIL(umount2_retry(tc->mntpoint, tc->flag), tc->exp_errno,
>> +			"umount2_retry(%s, %d)", tc->mntpoint, tc->flag);
>> +		if (!TST_PASS)
>> +			mount_flag = 0;
>> +	} else {
>> +		TST_EXP_PASS(umount2_retry(tc->mntpoint, tc->flag),
>> +			"umount2_retry(%s, %d)", tc->mntpoint, tc->flag);
>> +		if (TST_PASS)
>> +			mount_flag = 0;
>>   	}
> nit: this would be more compact:
>
> 	if (tc->exp_errno)
> 		TST_EXP_FAIL(umount2_retry(tc->mntpoint, tc->flag), tc->exp_errno,
> 			"umount2_retry(%s, %d)", tc->mntpoint, tc->flag);
> 	else
> 		TST_EXP_PASS(umount2_retry(tc->mntpoint, tc->flag),
> 			"umount2_retry(%s, %d)", tc->mntpoint, tc->flag);
>
> 	if (!!tc->exp_errno ^ !!TST_PASS)
> 		mount_flag = 0;
>
> Kind regards,
> Petr

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [LTP] [PATCH v5] syscalls/umount2: Convert to new API and use SAFE_ACCESS
  2022-03-25 17:29                       ` [LTP] [PATCH v5] " Dai Shili
@ 2022-03-25  8:46                         ` Petr Vorel
  0 siblings, 0 replies; 27+ messages in thread
From: Petr Vorel @ 2022-03-25  8:46 UTC (permalink / raw)
  To: Dai Shili; +Cc: ltp

Hi Dai,

> v4->v5:
>   - Add changelog from previous version
nit: You don't have mention in changelog that you're adding it.
(note anything after --- is not in the commit message)

>   - Make commit message order more reasonable
>   - Fix description format
>   - Merge umount2.h into umount2_02
>   - Use macro to not repeat flag and exp_errno
>   - Make test_umount2() more compact
Thanks for implementing all this!

I merged it with few minor changes, added Xu's Reviewed-by: tag
(he reviewed v2 and v3).

...
> +++ b/testcases/kernel/syscalls/umount2/umount2_02.c
> +#define FLAG_DESC(x, y) .flag = x, .exp_errno = 0, .desc = "umount2("y") with "#x" expected success"
> +#define FLAG_EXP_ERRNO_DESC(x, y, z) .flag = x, .exp_errno = y, \
> +				     .desc = "umount2("z") with "#x" expected "#y
I cleaned whitespace here.

...
> +	const char *mntpoint;
> +	int do_access;
> +} tcases[] = {
> +	{FLAG_EXP_ERRNO_DESC(MNT_EXPIRE | MNT_FORCE, EINVAL, ""), MNTPOINT, 0},

I removed blank lines in here.

...
> +static inline int umount2_retry(const char *target, int flags)
I removed this inline (required for *.h, but not needed here).

Kind regards,
Petr

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [LTP] [PATCH v5] syscalls/umount2: Convert to new API and use SAFE_ACCESS
  2022-03-24  9:11                     ` Petr Vorel
  2022-03-25  3:00                       ` daisl.fnst
@ 2022-03-25 17:29                       ` Dai Shili
  2022-03-25  8:46                         ` Petr Vorel
  1 sibling, 1 reply; 27+ messages in thread
From: Dai Shili @ 2022-03-25 17:29 UTC (permalink / raw)
  To: pvorel; +Cc: ltp

1. use SAFE macro
2. use TST_EXP_FAIL and TST_EXP_PASS macro
3. simplify verify operations
4. merge umount2_03 and umount2.h to umount2_02

Signed-off-by: Dai Shili <daisl.fnst@fujitsu.com>
---
v4->v5:
  - Add changelog from previous version
  - Make commit message order more reasonable
  - Fix description format
  - Merge umount2.h into umount2_02
  - Use macro to not repeat flag and exp_errno
  - Make test_umount2() more compact

 runtest/syscalls                               |   1 -
 testcases/kernel/syscalls/umount2/.gitignore   |   1 -
 testcases/kernel/syscalls/umount2/umount2.h    |  44 -----
 testcases/kernel/syscalls/umount2/umount2_02.c | 212 +++++++++----------------
 testcases/kernel/syscalls/umount2/umount2_03.c | 167 -------------------
 5 files changed, 77 insertions(+), 348 deletions(-)
 delete mode 100644 testcases/kernel/syscalls/umount2/umount2.h
 delete mode 100644 testcases/kernel/syscalls/umount2/umount2_03.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 6186bfc..0eea946 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1647,7 +1647,6 @@ umount03 umount03
 
 umount2_01 umount2_01
 umount2_02 umount2_02
-umount2_03 umount2_03
 
 userfaultfd01 userfaultfd01
 
diff --git a/testcases/kernel/syscalls/umount2/.gitignore b/testcases/kernel/syscalls/umount2/.gitignore
index 1d6a0e8..e2056bf 100644
--- a/testcases/kernel/syscalls/umount2/.gitignore
+++ b/testcases/kernel/syscalls/umount2/.gitignore
@@ -1,3 +1,2 @@
 /umount2_01
 /umount2_02
-/umount2_03
diff --git a/testcases/kernel/syscalls/umount2/umount2.h b/testcases/kernel/syscalls/umount2/umount2.h
deleted file mode 100644
index 65e4c24..0000000
--- a/testcases/kernel/syscalls/umount2/umount2.h
+++ /dev/null
@@ -1,44 +0,0 @@
-/*
- * Copyright (c) 2016 Cyril Hrubis <chrubis@suse.cz>
- *
- * 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, see <http://www.gnu.org/licenses/>.
- */
-
-#ifndef UMOUNT2_H__
-#define UMOUNT2_H__
-
-static inline int umount2_retry(const char *target, int flags)
-{
-	int i, ret;
-
-	for (i = 0; i < 50; i++) {
-		ret = umount2(target, flags);
-
-		if (ret == 0 || errno != EBUSY)
-			return ret;
-
-		tst_resm(TINFO, "umount('%s', %i) failed with EBUSY, try %2i...",
-			 target, flags, i);
-
-		usleep(100000);
-	}
-
-	tst_resm(TWARN, "Failed to umount('%s', %i) after 50 retries",
-	         target, flags);
-
-	errno = EBUSY;
-	return -1;
-}
-
-#endif	/* UMOUNT2_H__ */
diff --git a/testcases/kernel/syscalls/umount2/umount2_02.c b/testcases/kernel/syscalls/umount2/umount2_02.c
index 7d558fa..06cb36b 100644
--- a/testcases/kernel/syscalls/umount2/umount2_02.c
+++ b/testcases/kernel/syscalls/umount2/umount2_02.c
@@ -1,182 +1,124 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * Copyright (c) 2015 Fujitsu Ltd.
+ * Copyright (c) 2015-2022 FUJITSU LIMITED. All rights reserved
  * Author: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
+ */
+
+/*\
+ * [Description]
  *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
+ * Test for feature MNT_EXPIRE of umount2().
  *
- * This program is distributed in the hope that it would be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * - EINVAL when flag is specified with either MNT_FORCE or MNT_DETACH
+ * - EAGAIN when initial call to umount2(2) with MNT_EXPIRE
+ * - EAGAIN when umount2(2) with MNT_EXPIRE after access(2)
+ * - succeed when second call to umount2(2) with MNT_EXPIRE
  *
- * You should have received a copy of the GNU General Public License
- * alone with this program.
- */
-
-/*
- * DESCRIPTION
- *  Test for feature MNT_EXPIRE of umount2().
- *  "Mark the mount point as expired.If a mount point is not currently
- *   in use, then an initial call to umount2() with this flag fails with
- *   the error EAGAIN, but marks the mount point as expired. The mount
- *   point remains expired as long as it isn't accessed by any process.
- *   A second umount2() call specifying MNT_EXPIRE unmounts an expired
- *   mount point. This flag cannot be specified with either MNT_FORCE or
- *   MNT_DETACH. (fails with the error EINVAL)"
+ * Test for feature UMOUNT_NOFOLLOW of umount2().
+ *
+ * - EINVAL when target is a symbolic link
+ * - succeed when target is a mount point
  */
 
-#include <errno.h>
 #include <sys/mount.h>
-
-#include "test.h"
-#include "safe_macros.h"
 #include "lapi/mount.h"
+#include "tst_test.h"
 
-#include "umount2.h"
-
-#define DIR_MODE	(S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH)
-#define MNTPOINT	"mntpoint"
-
-static void setup(void);
-static void test_umount2(int i);
-static void verify_failure(int i);
-static void verify_success(int i);
-static void cleanup(void);
+#define MNTPOINT        "mntpoint"
+#define SYMLINK	"symlink"
 
-static const char *device;
-static const char *fs_type;
+#define FLAG_DESC(x, y) .flag = x, .exp_errno = 0, .desc = "umount2("y") with "#x" expected success"
+#define FLAG_EXP_ERRNO_DESC(x, y, z) .flag = x, .exp_errno = y, \
+				     .desc = "umount2("z") with "#x" expected "#y
 
 static int mount_flag;
 
-static struct test_case_t {
+static struct tcase {
 	int flag;
 	int exp_errno;
-	int do_access;
 	const char *desc;
-} test_cases[] = {
-	{MNT_EXPIRE | MNT_FORCE, EINVAL, 0,
-		"umount2(2) with MNT_EXPIRE | MNT_FORCE expected EINVAL"},
-	{MNT_EXPIRE | MNT_DETACH, EINVAL, 0,
-		"umount2(2) with MNT_EXPIRE | MNT_DETACH expected EINVAL"},
-	{MNT_EXPIRE, EAGAIN, 0,
-		"initial call to umount2(2) with MNT_EXPIRE expected EAGAIN"},
-	{MNT_EXPIRE, EAGAIN, 1,
-		"umount2(2) with MNT_EXPIRE after access(2) expected EAGAIN"},
-	{MNT_EXPIRE, 0, 0,
-		"second call to umount2(2) with MNT_EXPIRE expected success"},
-};
-
-char *TCID = "umount2_02";
-int TST_TOTAL = ARRAY_SIZE(test_cases);
-
-int main(int ac, char **av)
-{
-	int lc;
-	int tc;
-
-	tst_parse_opts(ac, av, NULL, NULL);
+	const char *mntpoint;
+	int do_access;
+} tcases[] = {
+	{FLAG_EXP_ERRNO_DESC(MNT_EXPIRE | MNT_FORCE, EINVAL, ""), MNTPOINT, 0},
 
-	setup();
+	{FLAG_EXP_ERRNO_DESC(MNT_EXPIRE | MNT_DETACH, EINVAL, ""), MNTPOINT, 0},
 
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
+	{FLAG_EXP_ERRNO_DESC(MNT_EXPIRE, EAGAIN, "initial call"), MNTPOINT, 0},
 
-		SAFE_MOUNT(cleanup, device, MNTPOINT, fs_type, 0, NULL);
-		mount_flag = 1;
+	{FLAG_EXP_ERRNO_DESC(MNT_EXPIRE, EAGAIN, "after access"), MNTPOINT, 1},
 
-		for (tc = 0; tc < TST_TOTAL; tc++)
-			test_umount2(tc);
+	{FLAG_DESC(MNT_EXPIRE, "second call"), MNTPOINT, 0},
 
-		if (mount_flag) {
-			if (tst_umount(MNTPOINT))
-				tst_brkm(TBROK, cleanup, "umount() failed");
-			mount_flag = 0;
-		}
-	}
+	{FLAG_EXP_ERRNO_DESC(UMOUNT_NOFOLLOW, EINVAL, "symlink"), SYMLINK, 0},
 
-	cleanup();
-	tst_exit();
-}
+	{FLAG_DESC(UMOUNT_NOFOLLOW, "mntpoint"), MNTPOINT, 0},
+};
 
-static void setup(void)
+static inline int umount2_retry(const char *target, int flags)
 {
-	tst_require_root();
+	int i;
 
-	if ((tst_kvercmp(2, 6, 8)) < 0) {
-		tst_brkm(TCONF, NULL, "This test can only run on kernels "
-			 "that are 2.6.8 or higher");
-	}
-
-	tst_sig(NOFORK, DEF_HANDLER, NULL);
-
-	tst_tmpdir();
+	for (i = 0; i < 50; i++) {
+		TEST(umount2(target, flags));
+		if (TST_RET == 0 || TST_ERR != EBUSY)
+			return TST_RET;
 
-	fs_type = tst_dev_fs_type();
-	device = tst_acquire_device(cleanup);
+		tst_res(TINFO, "umount('%s', %i) failed with EBUSY, try %2i...",
+			target, flags, i);
 
-	if (!device)
-		tst_brkm(TCONF, cleanup, "Failed to obtain block device");
-
-	tst_mkfs(cleanup, device, fs_type, NULL, NULL);
+		usleep(100000);
+	}
 
-	SAFE_MKDIR(cleanup, MNTPOINT, DIR_MODE);
+	tst_res(TWARN, "Failed to umount('%s', %i) after 50 retries",
+		target, flags);
 
-	TEST_PAUSE;
+	TST_ERR = EBUSY;
+	return -1;
 }
 
-static void test_umount2(int i)
+static void test_umount2(unsigned int n)
 {
-	/* a new access removes the expired mark of the mount point */
-	if (test_cases[i].do_access) {
-		if (access(MNTPOINT, F_OK) == -1)
-			tst_brkm(TBROK | TERRNO, cleanup, "access(2) failed");
+	struct tcase *tc = &tcases[n];
+
+	if (!mount_flag) {
+		SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, NULL);
+		mount_flag = 1;
 	}
 
-	TEST(umount2_retry(MNTPOINT, test_cases[i].flag));
+	tst_res(TINFO, "Testing %s", tc->desc);
+
+	if (tc->do_access)
+		SAFE_ACCESS(MNTPOINT, F_OK);
 
-	if (test_cases[i].exp_errno != 0)
-		verify_failure(i);
+	if (tc->exp_errno)
+		TST_EXP_FAIL(umount2_retry(tc->mntpoint, tc->flag), tc->exp_errno,
+			"umount2_retry(%s, %d)", tc->mntpoint, tc->flag);
 	else
-		verify_success(i);
-}
+		TST_EXP_PASS(umount2_retry(tc->mntpoint, tc->flag),
+			"umount2_retry(%s, %d)", tc->mntpoint, tc->flag);
 
-static void verify_failure(int i)
-{
-	if (TEST_RETURN == 0) {
-		tst_resm(TFAIL, "%s passed unexpectedly", test_cases[i].desc);
+	if (!!tc->exp_errno ^ !!TST_PASS)
 		mount_flag = 0;
-		return;
-	}
-
-	if (TEST_ERRNO != test_cases[i].exp_errno) {
-		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
-			 test_cases[i].desc);
-		return;
-	}
-
-	tst_resm(TPASS | TTERRNO, "umount2(2) failed as expected");
 }
 
-static void verify_success(int i)
+static void setup(void)
 {
-	if (TEST_RETURN != 0) {
-		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
-			 test_cases[i].desc);
-		return;
-	}
-
-	tst_resm(TPASS, "umount2(2) succeeded as expected");
-	mount_flag = 0;
+	SAFE_SYMLINK(MNTPOINT, SYMLINK);
 }
 
 static void cleanup(void)
 {
-	if (mount_flag && tst_umount(MNTPOINT))
-		tst_resm(TWARN | TERRNO, "Failed to unmount");
-
-	if (device)
-		tst_release_device(device);
-
-	tst_rmdir();
+	if (mount_flag)
+		SAFE_UMOUNT(MNTPOINT);
 }
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.cleanup = cleanup,
+	.setup = setup,
+	.needs_root = 1,
+	.format_device = 1,
+	.mntpoint = MNTPOINT,
+	.test = test_umount2,
+};
diff --git a/testcases/kernel/syscalls/umount2/umount2_03.c b/testcases/kernel/syscalls/umount2/umount2_03.c
deleted file mode 100644
index a8fddf6..0000000
--- a/testcases/kernel/syscalls/umount2/umount2_03.c
+++ /dev/null
@@ -1,167 +0,0 @@
-/*
- * Copyright (c) 2015 Fujitsu Ltd.
- * Author: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- *
- * You should have received a copy of the GNU General Public License
- * alone with this program.
- */
-
-/*
- * DESCRIPTION
- *  Test for feature UMOUNT_NOFOLLOW of umount2().
- *  "Don't dereference target if it is a symbolic link,
- *   and fails with the error EINVAL."
- */
-
-#include <errno.h>
-#include <sys/mount.h>
-
-#include "test.h"
-#include "safe_macros.h"
-#include "lapi/mount.h"
-
-#include "umount2.h"
-
-#define DIR_MODE	(S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH)
-#define MNTPOINT	"mntpoint"
-#define SYMLINK	"symlink"
-
-static void setup(void);
-static void test_umount2(int i);
-static void verify_failure(int i);
-static void verify_success(int i);
-static void cleanup(void);
-
-static const char *device;
-static const char *fs_type;
-
-static int mount_flag;
-
-static struct test_case_t {
-	const char *mntpoint;
-	int exp_errno;
-	const char *desc;
-} test_cases[] = {
-	{SYMLINK, EINVAL,
-		"umount2('symlink', UMOUNT_NOFOLLOW) expected EINVAL"},
-	{MNTPOINT, 0,
-		"umount2('mntpoint', UMOUNT_NOFOLLOW) expected success"},
-};
-
-char *TCID = "umount2_03";
-int TST_TOTAL = ARRAY_SIZE(test_cases);
-
-int main(int ac, char **av)
-{
-	int lc;
-	int tc;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-
-		for (tc = 0; tc < TST_TOTAL; tc++)
-			test_umount2(tc);
-	}
-
-	cleanup();
-	tst_exit();
-}
-
-static void setup(void)
-{
-	tst_require_root();
-
-	if ((tst_kvercmp(2, 6, 34)) < 0) {
-		tst_brkm(TCONF, NULL, "This test can only run on kernels "
-			 "that are 2.6.34 or higher");
-	}
-
-	tst_sig(NOFORK, DEF_HANDLER, NULL);
-
-	tst_tmpdir();
-
-	fs_type = tst_dev_fs_type();
-	device = tst_acquire_device(cleanup);
-
-	if (!device)
-		tst_brkm(TCONF, cleanup, "Failed to obtain block device");
-
-	tst_mkfs(cleanup, device, fs_type, NULL, NULL);
-
-	SAFE_MKDIR(cleanup, MNTPOINT, DIR_MODE);
-
-	SAFE_SYMLINK(cleanup, MNTPOINT, SYMLINK);
-
-	TEST_PAUSE;
-}
-
-static void test_umount2(int i)
-{
-	SAFE_MOUNT(cleanup, device, MNTPOINT, fs_type, 0, NULL);
-	mount_flag = 1;
-
-	TEST(umount2_retry(test_cases[i].mntpoint, UMOUNT_NOFOLLOW));
-
-	if (test_cases[i].exp_errno != 0)
-		verify_failure(i);
-	else
-		verify_success(i);
-
-	if (mount_flag) {
-		if (tst_umount(MNTPOINT))
-			tst_brkm(TBROK, cleanup, "umount() failed");
-		mount_flag = 0;
-	}
-}
-
-static void verify_failure(int i)
-{
-	if (TEST_RETURN == 0) {
-		tst_resm(TFAIL, "%s passed unexpectedly", test_cases[i].desc);
-		mount_flag = 0;
-		return;
-	}
-
-	if (TEST_ERRNO != test_cases[i].exp_errno) {
-		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
-			 test_cases[i].desc);
-		return;
-	}
-
-	tst_resm(TPASS | TTERRNO, "umount2(2) failed as expected");
-}
-
-static void verify_success(int i)
-{
-	if (TEST_RETURN != 0) {
-		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
-			 test_cases[i].desc);
-		return;
-	}
-
-	tst_resm(TPASS, "umount2(2) succeeded as expected");
-	mount_flag = 0;
-}
-
-static void cleanup(void)
-{
-	if (mount_flag && tst_umount(MNTPOINT))
-		tst_resm(TWARN | TERRNO, "Failed to unmount");
-
-	if (device)
-		tst_release_device(device);
-
-	tst_rmdir();
-}
-- 
1.8.3.1


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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2022-03-25  8:47 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-14 17:28 [LTP] [PATCH 1/2] Add SAFE_ACCESS macro Dai Shili
2022-03-14 17:28 ` [LTP] [PATCH 2/2] Make use of SAFE_ACCESS Dai Shili
2022-03-14  6:43   ` xuyang2018.jy
2022-03-17 21:00     ` [LTP] [PATCH v2 1/3] Add SAFE_ACCESS macro Dai Shili
2022-03-17 21:00       ` [LTP] [PATCH v2 2/3] Make use of SAFE_ACCESS Dai Shili
2022-03-18  4:02         ` xuyang2018.jy
2022-03-17 21:00       ` [LTP] [PATCH v2 3/3] syscalls/umount2: Convert to new API and use SAFE_ACCESS Dai Shili
2022-03-18  4:03         ` xuyang2018.jy
2022-03-18  5:11           ` xuyang2018.jy
2022-03-18 18:55             ` [LTP] [PATCH v3 1/3] Add SAFE_ACCESS macro Dai Shili
2022-03-18 18:55               ` [LTP] [PATCH v3 2/3] Make use of SAFE_ACCESS Dai Shili
2022-03-23 19:30                 ` Petr Vorel
2022-03-23 19:33                 ` Petr Vorel
2022-03-18 18:55               ` [LTP] [PATCH v3 3/3] syscalls/umount2: Convert to new API and use SAFE_ACCESS Dai Shili
2022-03-23 19:36                 ` Petr Vorel
2022-03-24  2:23                   ` daisl.fnst
2022-03-24 15:35                   ` [LTP] [PATCH v4] " Dai Shili
2022-03-24  9:11                     ` Petr Vorel
2022-03-25  3:00                       ` daisl.fnst
2022-03-25 17:29                       ` [LTP] [PATCH v5] " Dai Shili
2022-03-25  8:46                         ` Petr Vorel
2022-03-24  1:37                 ` [LTP] [PATCH v3 3/3] " xuyang2018.jy
2022-03-23 19:24               ` [LTP] [PATCH v3 1/3] Add SAFE_ACCESS macro Petr Vorel
2022-03-24  2:22                 ` daisl.fnst
2022-03-23 19:27               ` Petr Vorel
2022-03-23 19:33               ` Petr Vorel
2022-03-18  3:56       ` [LTP] [PATCH v2 " xuyang2018.jy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.