[LTP] [PATCH v2 0/3] fanotify{14,20}: cleanup

[LTP] [PATCH v2 0/3] fanotify{14,20}: cleanup

[Petr Vorel]

Hi,

changes v1->v2:
* Rename macro (Cyril)
s/TST_EXP_FD_ERRNO/TST_EXP_FD_OR_FAIL/

Kind regards,
Petr

Petr Vorel (3):
  tst_test_macros: Add TST_EXP_FD_OR_FAIL() macro
  fanotify20: Simplify code
  fanotify14: Use TST_EXP_FD_ERRNO()

 include/tst_test_macros.h                     |  10 ++
 .../kernel/syscalls/fanotify/fanotify14.c     | 118 +++++-------------
 .../kernel/syscalls/fanotify/fanotify20.c     |  81 +++---------
 3 files changed, 62 insertions(+), 147 deletions(-)

-- 
2.37.3


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

[LTP] [PATCH v2 1/3] tst_test_macros: Add TST_EXP_FD_OR_FAIL() macro

[Petr Vorel]

It calls TST_EXP_FD() or TST_EXP_FAIL().

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 include/tst_test_macros.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/tst_test_macros.h b/include/tst_test_macros.h
index 2e7b7871c..231c04951 100644
--- a/include/tst_test_macros.h
+++ b/include/tst_test_macros.h
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) 2015-2020 Cyril Hrubis <chrubis@suse.cz>
+ * Copyright (c) Linux Test Project, 2021-2022
  */
 
 #ifndef TST_TEST_MACROS_H__
@@ -93,6 +94,15 @@ extern void *TST_RET_PTR;
 				#SCALL, ##__VA_ARGS__);                        \
 	} while (0)
 
+#define TST_EXP_FD_OR_FAIL(SCALL, ERRNO, ...)                                    \
+	do {                                                                   \
+		if (ERRNO)                                                     \
+			TST_EXP_FAIL(SCALL, ERRNO, ##__VA_ARGS__);             \
+		else                                                           \
+			TST_EXP_FD(SCALL, ##__VA_ARGS__);                      \
+		                                                               \
+	} while (0)
+
 #define TST_EXP_PID_SILENT(SCALL, ...)	TST_EXP_POSITIVE_(SCALL, #SCALL, ##__VA_ARGS__)
 
 #define TST_EXP_PID(SCALL, ...)                                                \
-- 
2.37.3


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

[LTP] [PATCH v2 2/3] fanotify20: Simplify code

[Petr Vorel]

* replace do_test() content with TST_EXP_FD_OR_FAIL() macro
* rename variables (shorten, use LTP common names)
* remove tc->want_err (not needed)
* add macro FLAGS_DESC (stringify)
* don't print number of tests (not needed for just 2 tests)

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 .../kernel/syscalls/fanotify/fanotify20.c     | 81 +++++--------------
 1 file changed, 19 insertions(+), 62 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
index de0fdb782..71310fb86 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify20.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) 2021 Google. All Rights Reserved.
+ * Copyright (c) 2022 Petr Vorel <pvorel@suse.cz>
  *
  * Started by Matthew Bobrowski <repnop@google.com>
  */
@@ -25,26 +26,21 @@
 #include "fanotify.h"
 
 #define MOUNT_PATH	"fs_mnt"
+#define FLAGS_DESC(x) .flags = x, .desc = #x
 
-static int fanotify_fd;
+static int fd;
 
 static struct test_case_t {
-	char *name;
-	unsigned int init_flags;
-	int want_err;
-	int want_errno;
+	unsigned int flags;
+	char *desc;
+	int exp_errno;
 } test_cases[] = {
 	{
-		"fail on FAN_REPORT_PIDFD | FAN_REPORT_TID",
-		FAN_REPORT_PIDFD | FAN_REPORT_TID,
-		1,
-		EINVAL,
+		FLAGS_DESC(FAN_REPORT_PIDFD | FAN_REPORT_TID),
+		.exp_errno = EINVAL,
 	},
 	{
-		"pass on FAN_REPORT_PIDFD | FAN_REPORT_FID | FAN_REPORT_DFID_NAME",
-		FAN_REPORT_PIDFD | FAN_REPORT_FID | FAN_REPORT_DFID_NAME,
-		0,
-		0,
+		FLAGS_DESC(FAN_REPORT_PIDFD | FAN_REPORT_FID | FAN_REPORT_DFID_NAME),
 	},
 };
 
@@ -57,63 +53,24 @@ static void do_setup(void)
 	REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_BY_KERNEL(FAN_REPORT_PIDFD);
 }
 
-static void do_test(unsigned int num)
+static void do_test(unsigned int i)
 {
-	struct test_case_t *tc = &test_cases[num];
+	struct test_case_t *tc = &test_cases[i];
 
-	tst_res(TINFO, "Test #%d: %s", num, tc->name);
+	tst_res(TINFO, "Test %s on %s", tc->exp_errno ? "fail" : "pass",
+		tc->desc);
 
-	fanotify_fd = fanotify_init(tc->init_flags, O_RDONLY);
-	if (fanotify_fd < 0) {
-		if (!tc->want_err) {
-			tst_res(TFAIL,
-				"fanotify_fd=%d, fanotify_init(%x, O_RDONLY) "
-				"failed with error -%d but wanted success",
-				fanotify_fd, tc->init_flags, errno);
-			return;
-		}
+	TST_EXP_FD_OR_FAIL(fd = fanotify_init(tc->flags, O_RDONLY),
+			   tc->exp_errno);
 
-		if (errno != tc->want_errno) {
-			tst_res(TFAIL,
-				"fanotify_fd=%d, fanotify_init(%x, O_RDONLY) "
-				"failed with an unexpected error code -%d but "
-				"wanted -%d",
-				fanotify_fd, tc->init_flags,
-				errno, tc->want_errno);
-			return;
-		}
-
-		tst_res(TPASS,
-			"fanotify_fd=%d, fanotify_init(%x, O_RDONLY) "
-			"failed with error -%d as expected",
-			fanotify_fd, tc->init_flags, errno);
-		return;
-	}
-
-	/*
-	 * Catch test cases that had expected to receive an error upon calling
-	 * fanotify_init() but had unexpectedly resulted in a success.
-	 */
-	if (tc->want_err) {
-		tst_res(TFAIL,
-			"fanotify_fd=%d, fanotify_init(%x, O_RDONLY) "
-			"unexpectedly returned successfully, wanted error -%d",
-			fanotify_fd, tc->init_flags, tc->want_errno);
-		return;
-	}
-
-	tst_res(TPASS,
-		"fanotify_fd=%d, fanotify_init(%x, O_RDONLY) "
-		"successfully initialized notification group",
-		fanotify_fd, tc->init_flags);
-
-	SAFE_CLOSE(fanotify_fd);
+	if (fd > 0)
+		SAFE_CLOSE(fd);
 }
 
 static void do_cleanup(void)
 {
-	if (fanotify_fd >= 0)
-		SAFE_CLOSE(fanotify_fd);
+	if (fd > 0)
+		SAFE_CLOSE(fd);
 }
 
 static struct tst_test test = {
-- 
2.37.3


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

[LTP] [PATCH v2 3/3] fanotify14: Use TST_EXP_FD_ERRNO()

[Petr Vorel]

That greatly simplifies the code.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 .../kernel/syscalls/fanotify/fanotify14.c     | 118 +++++-------------
 1 file changed, 33 insertions(+), 85 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c
index 4a1953b33..594259ccf 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify14.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify14.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
  * Copyright (c) 2018 Matthew Bobrowski. All Rights Reserved.
+ * Copyright (c) Linux Test Project, 2020-2022
  *
  * Started by Matthew Bobrowski <mbobrowski@mbobrowski.org>
  */
@@ -49,6 +50,7 @@ static int ignore_mark_unsupported;
 static struct test_case_t {
 	unsigned int init_flags;
 	unsigned int mark_flags;
+	/* zero mask expects to fail on fanotify_init() */
 	unsigned long long mask;
 	int expected_errno;
 } test_cases[] = {
@@ -140,7 +142,6 @@ static struct test_case_t {
 
 static void do_test(unsigned int number)
 {
-	int ret;
 	struct test_case_t *tc = &test_cases[number];
 
 	if (fan_report_target_fid_unsupported && tc->init_flags & FAN_REPORT_TARGET_FID) {
@@ -154,101 +155,48 @@ static void do_test(unsigned int number)
 		return;
 	}
 
-	fanotify_fd = fanotify_init(tc->init_flags, O_RDONLY);
-	if (fanotify_fd < 0) {
-		if (errno == tc->expected_errno) {
-			tst_res(TPASS,
-				"fanotify_fd=%d, fanotify_init(%x, O_RDONLY) "
-				"failed with error %d as expected",
-				fanotify_fd,
-				tc->init_flags, tc->expected_errno);
-			return;
-		}
-		tst_brk(TBROK | TERRNO,
-			"fanotify_fd=%d, fanotify_init(%x, O_RDONLY) failed",
-			fanotify_fd,
-			tc->init_flags);
-	}
+	TST_EXP_FD_OR_FAIL(fanotify_fd = fanotify_init(tc->init_flags, O_RDONLY),
+			   !tc->mask && tc->expected_errno ? tc->expected_errno : 0);
 
-	/*
-	 * A test case with a mask set to zero indicate that they've been
-	 * specifically designed to test and fail on the fanotify_init()
-	 * system call.
-	 */
-	if (tc->mask == 0) {
-		tst_res(TFAIL,
-			"fanotify_fd=%d fanotify_init(%x, O_RDONLY) "
-			"unexpectedly succeeded when tests with mask 0 are "
-			"expected to fail when calling fanotify_init()",
-			fanotify_fd,
-			tc->init_flags);
+	if (fanotify_fd < 0)
+		return;
+
+	if (!tc->mask)
 		goto out;
-	}
 
 	/* Set mark on non-dir only when expecting error ENOTDIR */
 	const char *path = tc->expected_errno == ENOTDIR ? FILE1 : MNTPOINT;
 
-	ret = fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark_flags,
-				tc->mask, AT_FDCWD, path);
-	if (ret < 0) {
-		if (errno == tc->expected_errno) {
-			tst_res(TPASS,
-				"ret=%d, fanotify_mark(%d, FAN_MARK_ADD | %x, "
-				"%llx, AT_FDCWD, %s) failed with error %d "
-				"as expected",
-				ret,
-				fanotify_fd,
-				tc->mark_flags,
-				tc->mask,
-				path, tc->expected_errno);
-			/*
-			 * ENOTDIR are errors for events/flags not allowed on a non-dir inode.
-			 * Try to set an inode mark on a directory and it should succeed.
-			 * Try to set directory events in filesystem mark mask on non-dir
-			 * and it should succeed.
-			 */
-			if (tc->expected_errno == ENOTDIR) {
-				SAFE_FANOTIFY_MARK(fanotify_fd, FAN_MARK_ADD | tc->mark_flags,
-						   tc->mask, AT_FDCWD, MNTPOINT);
-				tst_res(TPASS,
-					"Adding an inode mark on directory did not fail with "
-					"ENOTDIR error as on non-dir inode");
-			}
-			if (tc->expected_errno == ENOTDIR &&
-			    !(tc->mark_flags & FAN_MARK_ONLYDIR)) {
-				SAFE_FANOTIFY_MARK(fanotify_fd, FAN_MARK_ADD | tc->mark_flags |
-						   FAN_MARK_FILESYSTEM, tc->mask,
-						   AT_FDCWD, FILE1);
-				tst_res(TPASS,
-					"Adding a filesystem mark on non-dir did not fail with "
-					"ENOTDIR error as with an inode mark");
-			}
+	TST_EXP_FD_OR_FAIL(fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark_flags,
+					 tc->mask, AT_FDCWD, path),
+					 tc->expected_errno);
 
-			goto out;
+	/*
+	 * ENOTDIR are errors for events/flags not allowed on a non-dir inode.
+	 * Try to set an inode mark on a directory and it should succeed.
+	 * Try to set directory events in filesystem mark mask on non-dir
+	 * and it should succeed.
+	 */
+	if (TST_PASS && tc->expected_errno == ENOTDIR) {
+		SAFE_FANOTIFY_MARK(fanotify_fd, FAN_MARK_ADD | tc->mark_flags,
+				   tc->mask, AT_FDCWD, MNTPOINT);
+		tst_res(TPASS,
+			"Adding an inode mark on directory did not fail with "
+			"ENOTDIR error as on non-dir inode");
+
+		if (!(tc->mark_flags & FAN_MARK_ONLYDIR)) {
+			SAFE_FANOTIFY_MARK(fanotify_fd, FAN_MARK_ADD | tc->mark_flags |
+					   FAN_MARK_FILESYSTEM, tc->mask,
+					   AT_FDCWD, FILE1);
+			tst_res(TPASS,
+				"Adding a filesystem mark on non-dir did not fail with "
+				"ENOTDIR error as with an inode mark");
 		}
-		tst_brk(TBROK | TERRNO,
-			"ret=%d, fanotify_mark(%d, FAN_MARK_ADD | %x, %llx, "
-			"AT_FDCWD, %s) failed",
-			ret,
-			fanotify_fd,
-			tc->mark_flags,
-			tc->mask,
-			FILE1);
 	}
 
-	tst_res(TFAIL,
-		"fanotify_fd=%d, ret=%d, fanotify_init(%x, O_RDONLY) and "
-		"fanotify_mark(%d, FAN_MARK_ADD | %x, %llx, AT_FDCWD, %s) did "
-		"not return any errors as expected",
-		fanotify_fd,
-		ret,
-		tc->init_flags,
-		fanotify_fd,
-		tc->mark_flags,
-		tc->mask,
-		FILE1);
 out:
-	SAFE_CLOSE(fanotify_fd);
+	if (fanotify_fd > 0)
+		SAFE_CLOSE(fanotify_fd);
 }
 
 static void do_setup(void)
-- 
2.37.3


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

Re: [LTP] [PATCH v2 0/3] fanotify{14,20}: cleanup

[Petr Vorel]

> Hi,

> changes v1->v2:
> * Rename macro (Cyril)
> s/TST_EXP_FD_ERRNO/TST_EXP_FD_OR_FAIL/

Forget to mention, that Amir and Matthew acked 2nd and 3rd patch in v1.

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 3/3] fanotify14: Use TST_EXP_FD_ERRNO()

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 125 bytes --]

Hi Petr,

The TST_EXP_FD_ERRNO in the subject should be corrected as well.
Otherwise, patch set LGTM.


-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 545 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


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

Re: [LTP] [PATCH v2 3/3] fanotify14: Use TST_EXP_FD_ERRNO()

[Petr Vorel]

Hi Li,

> Hi Petr,

> The TST_EXP_FD_ERRNO in the subject should be corrected as well.
> Otherwise, patch set LGTM.

Thanks for catching this. V2 merged.

Kind regards,
Petr

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