[LTP] [PATCH v3 0/5] Introduce SAFE_FANOTIFY_MARK() macro + cleanup

[LTP] [PATCH v3 0/5] Introduce SAFE_FANOTIFY_MARK() macro + cleanup

[Petr Vorel]

Hi Amir,

quick fix for old distros.

changes v2->v3:
* move safe_fanotify_*() after constant definitions

Kind regards,
Petr

Petr Vorel (5):
  fanotify12: Drop incorrect hint
  fanotify: Handle supported features checks in setup()
  fanotify: Introduce SAFE_FANOTIFY_MARK() macro
  fanotify: Check FAN_REPORT_{FID,NAME} support
  fanotify: Add a pedantic check for return value

 testcases/kernel/syscalls/fanotify/fanotify.h | 150 +++++++++++++++---
 .../kernel/syscalls/fanotify/fanotify01.c     |  53 ++-----
 .../kernel/syscalls/fanotify/fanotify02.c     |  22 +--
 .../kernel/syscalls/fanotify/fanotify03.c     |  48 ++----
 .../kernel/syscalls/fanotify/fanotify04.c     |  32 +---
 .../kernel/syscalls/fanotify/fanotify05.c     |   9 +-
 .../kernel/syscalls/fanotify/fanotify06.c     |  21 +--
 .../kernel/syscalls/fanotify/fanotify07.c     |  17 +-
 .../kernel/syscalls/fanotify/fanotify09.c     |  19 +--
 .../kernel/syscalls/fanotify/fanotify10.c     |  44 ++---
 .../kernel/syscalls/fanotify/fanotify11.c     |   5 +-
 .../kernel/syscalls/fanotify/fanotify12.c     |  57 ++-----
 .../kernel/syscalls/fanotify/fanotify13.c     |  33 +---
 .../kernel/syscalls/fanotify/fanotify15.c     |  24 +--
 .../kernel/syscalls/fanotify/fanotify16.c     |  20 +--
 15 files changed, 209 insertions(+), 345 deletions(-)

-- 
2.29.2

[LTP] [PATCH v3 1/5] fanotify12: Drop incorrect hint

[Petr Vorel]

hint "CONFIG_FANOTIFY_ACCESS_PERMISSIONS not configured in kernel?"
is wrong here (fanotify12 does not use FAN_ACCESS_PERM).

Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/syscalls/fanotify/fanotify12.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify12.c b/testcases/kernel/syscalls/fanotify/fanotify12.c
index fcb7ec0d3..7f23fc9dc 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify12.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify12.c
@@ -146,10 +146,6 @@ static int setup_mark(unsigned int n)
 					"FAN_OPEN_EXEC not supported in "
 					"kernel?");
 				return -1;
-			} else if (errno == EINVAL) {
-				tst_brk(TCONF | TERRNO,
-					"CONFIG_FANOTIFY_ACCESS_PERMISSIONS "
-					"not configured in kernel?");
 			}else {
 				tst_brk(TBROK | TERRNO,
 					"fanotify_mark(%d, FAN_MARK_ADD | %s, "
-- 
2.29.2

[LTP] [PATCH v3 1/5] fanotify12: Drop incorrect hint

[Cyril Hrubis]

Hi!
> hint "CONFIG_FANOTIFY_ACCESS_PERMISSIONS not configured in kernel?"
> is wrong here (fanotify12 does not use FAN_ACCESS_PERM).
> 
> Suggested-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>  testcases/kernel/syscalls/fanotify/fanotify12.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify12.c b/testcases/kernel/syscalls/fanotify/fanotify12.c
> index fcb7ec0d3..7f23fc9dc 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify12.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify12.c
> @@ -146,10 +146,6 @@ static int setup_mark(unsigned int n)
>  					"FAN_OPEN_EXEC not supported in "
>  					"kernel?");
>  				return -1;
> -			} else if (errno == EINVAL) {
> -				tst_brk(TCONF | TERRNO,
> -					"CONFIG_FANOTIFY_ACCESS_PERMISSIONS "
> -					"not configured in kernel?");
>  			}else {
>  				tst_brk(TBROK | TERRNO,
>  					"fanotify_mark(%d, FAN_MARK_ADD | %s, "

Looking at the source there is the same message just couple of lines
below this one, should we get rid of that one as well?

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3 2/5] fanotify: Handle supported features checks in setup()

[Petr Vorel]

Create 2 helpers which are used in various tests in setup() to check for
FAN_ACCESS_PERM enablement (caused by disabled CONFIG_FANOTIFY_ACCESS_PERMISSIONS)
or FAN_OPEN_EXEC and FAN_OPEN_EXEC_PERM support in kernel.

That helps to further code simplification in next commit.

Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/syscalls/fanotify/fanotify.h | 47 +++++++++++++++
 .../kernel/syscalls/fanotify/fanotify03.c     | 30 ++++------
 .../kernel/syscalls/fanotify/fanotify07.c     |  2 +
 .../kernel/syscalls/fanotify/fanotify10.c     |  8 +++
 .../kernel/syscalls/fanotify/fanotify12.c     | 57 ++++++++-----------
 5 files changed, 91 insertions(+), 53 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 0afbc02aa..0c61f8ab7 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -242,4 +242,51 @@ static inline void fanotify_save_fid(const char *path,
 #define INIT_FANOTIFY_MARK_TYPE(t) \
 	{ FAN_MARK_ ## t, "FAN_MARK_" #t }
 
+
+static inline void fanotify_access_permissions_supported_by_kernel(void)
+{
+	int fd;
+
+	fd = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
+
+	if (fanotify_mark(fd, FAN_MARK_ADD, FAN_ACCESS_PERM, AT_FDCWD, ".") < 0) {
+		if (errno == EINVAL) {
+			tst_brk(TCONF | TERRNO,
+				"CONFIG_FANOTIFY_ACCESS_PERMISSIONS not configured in kernel?");
+		} else {
+			tst_brk(TBROK | TERRNO,
+				"fanotify_mark (%d, FAN_MARK_ADD, FAN_ACCESS_PERM, AT_FDCWD,"
+				" \".\") failed", fd);
+		}
+	}
+
+	SAFE_CLOSE(fd);
+}
+
+static inline int fanotify_exec_events_supported_by_kernel(uint64_t mask,
+							   const char* smask)
+{
+	int fd;
+	int rval = 0;
+
+	fd = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
+
+	if (fanotify_mark(fd, FAN_MARK_ADD, mask, AT_FDCWD, ".") < 0) {
+		if (errno == EINVAL) {
+			rval = -1;
+		} else {
+			tst_brk(TBROK | TERRNO,
+				"fanotify_mark (%d, FAN_MARK_ADD, %s, AT_FDCWD, \".\") failed",
+				fd, smask);
+		}
+	}
+
+	SAFE_CLOSE(fd);
+
+	return rval;
+}
+
+#define FANOTIFY_EXEC_EVENTS_SUPPORTED_BY_KERNEL(mask) \
+	fanotify_exec_events_supported_by_kernel(mask, #mask)
+
 #endif /* __FANOTIFY_H__ */
diff --git a/testcases/kernel/syscalls/fanotify/fanotify03.c b/testcases/kernel/syscalls/fanotify/fanotify03.c
index 1ef1c206b..fbec652f6 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify03.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify03.c
@@ -212,28 +212,23 @@ static int setup_mark(unsigned int n)
 	char *const files[] = {fname, FILE_EXEC_PATH};
 
 	tst_res(TINFO, "Test #%d: %s", n, tc->tname);
+
+	if (support_exec_events != 0 && tc->mask & FAN_OPEN_EXEC_PERM) {
+		tst_res(TCONF | TERRNO, "FAN_OPEN_EXEC_PERM not supported in kernel?");
+		return -1;
+	}
+
 	fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
 
 	for (; i < ARRAY_SIZE(files); i++) {
 		if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
 				  tc->mask, AT_FDCWD, files[i]) < 0) {
 			if (errno == EINVAL &&
-				(tc->mask & FAN_OPEN_EXEC_PERM &&
-				 !support_exec_events)) {
-				tst_res(TCONF,
-					"FAN_OPEN_EXEC_PERM not supported in "
-					"kernel?");
-				return -1;
-			} else if (errno == EINVAL &&
 					mark->flag == FAN_MARK_FILESYSTEM) {
 				tst_res(TCONF,
 					"FAN_MARK_FILESYSTEM not supported in "
 					"kernel?");
 				return -1;
-			} else if (errno == EINVAL) {
-				tst_brk(TCONF | TERRNO,
-					"CONFIG_FANOTIFY_ACCESS_PERMISSIONS "
-					"not configured in kernel?");
 			} else {
 				tst_brk(TBROK | TERRNO,
 					"fanotify_mark(%d, FAN_MARK_ADD | %s, "
@@ -241,14 +236,6 @@ static int setup_mark(unsigned int n)
 					"AT_FDCWD, %s) failed.",
 					fd_notify, mark->name, fname);
 			}
-		} else {
-			/*
-			 * To distinguish between perm not supported, exec
-			 * events not supported and filesystem mark not
-			 * supported.
-			 */
-			if (tc->mask & FAN_OPEN_EXEC_PERM)
-				support_exec_events = 1;
 		}
 	}
 
@@ -347,6 +334,11 @@ static void test_fanotify(unsigned int n)
 
 static void setup(void)
 {
+
+	fanotify_access_permissions_supported_by_kernel();
+
+	support_exec_events = FANOTIFY_EXEC_EVENTS_SUPPORTED_BY_KERNEL(FAN_OPEN_EXEC_PERM);
+
 	sprintf(fname, MOUNT_PATH"/fname_%d", getpid());
 	SAFE_FILE_PRINTF(fname, "1");
 
diff --git a/testcases/kernel/syscalls/fanotify/fanotify07.c b/testcases/kernel/syscalls/fanotify/fanotify07.c
index c2e185710..f4e8ac9e6 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify07.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify07.c
@@ -195,6 +195,8 @@ static void test_fanotify(void)
 
 static void setup(void)
 {
+	fanotify_access_permissions_supported_by_kernel();
+
 	sprintf(fname, "fname_%d", getpid());
 	SAFE_FILE_PRINTF(fname, "%s", fname);
 }
diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
index 90cf5cb5f..b95efb998 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify10.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
@@ -64,6 +64,7 @@ static unsigned int fanotify_class[] = {
 static int fd_notify[NUM_CLASSES][GROUPS_PER_PRIO];
 
 static char event_buf[EVENT_BUF_LEN];
+static int support_exec_events;
 
 #define MOUNT_PATH "fs_mnt"
 #define MNT2_PATH "mntpoint"
@@ -451,6 +452,11 @@ static void test_fanotify(unsigned int n)
 
 	tst_res(TINFO, "Test #%d: %s", n, tc->tname);
 
+	if (support_exec_events != 0 && tc->expected_mask_with_ignore & FAN_OPEN_EXEC) {
+		tst_res(TCONF | TERRNO, "FAN_OPEN_EXEC not supported in kernel?");
+		return;
+	}
+
 	if (tc->ignored_onchild && tst_kvercmp(5, 9, 0) < 0) {
 		tst_res(TCONF, "ignored mask in combination with flag FAN_EVENT_ON_CHILD"
 				" has undefined behavior on kernel < 5.9");
@@ -535,6 +541,8 @@ cleanup:
 
 static void setup(void)
 {
+	support_exec_events = FANOTIFY_EXEC_EVENTS_SUPPORTED_BY_KERNEL(FAN_OPEN_EXEC);
+
 	/* Create another bind mount@another path for generating events */
 	SAFE_MKDIR(MNT2_PATH, 0755);
 	SAFE_MOUNT(MOUNT_PATH, MNT2_PATH, "none", MS_BIND, NULL);
diff --git a/testcases/kernel/syscalls/fanotify/fanotify12.c b/testcases/kernel/syscalls/fanotify/fanotify12.c
index 7f23fc9dc..bddbdc37c 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify12.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify12.c
@@ -39,6 +39,7 @@ static char fname[BUF_SIZE];
 static volatile int fd_notify;
 static volatile int complete;
 static char event_buf[EVENT_BUF_LEN];
+static int support_exec_events;
 
 static struct test_case_t {
 	const char *tname;
@@ -135,26 +136,25 @@ static int setup_mark(unsigned int n)
 	const char *const files[] = {fname, TEST_APP};
 
 	tst_res(TINFO, "Test #%d: %s", n, tc->tname);
+
+	if (support_exec_events != 0 && tc->mask & FAN_OPEN_EXEC) {
+		tst_res(TCONF | TERRNO, "FAN_OPEN_EXEC not supported in kernel?");
+		return -1;
+	}
+
 	fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
 
 	for (; i < ARRAY_SIZE(files); i++) {
 		/* Setup normal mark on object */
 		if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
 					tc->mask, AT_FDCWD, files[i]) < 0) {
-			if (errno == EINVAL && tc->mask & FAN_OPEN_EXEC) {
-				tst_res(TCONF,
-					"FAN_OPEN_EXEC not supported in "
-					"kernel?");
-				return -1;
-			}else {
-				tst_brk(TBROK | TERRNO,
-					"fanotify_mark(%d, FAN_MARK_ADD | %s, "
-					"%llx, AT_FDCWD, %s) failed",
-					fd_notify,
-					mark->name,
-					tc->mask,
-					files[i]);
-			}
+			tst_brk(TBROK | TERRNO,
+				"fanotify_mark(%d, FAN_MARK_ADD | %s, "
+				"%llx, AT_FDCWD, %s) failed",
+				fd_notify,
+				mark->name,
+				tc->mask,
+				files[i]);
 		}
 
 		/* Setup ignore mark on object */
@@ -163,26 +163,13 @@ static int setup_mark(unsigned int n)
 						| FAN_MARK_IGNORED_MASK,
 						tc->ignore_mask, AT_FDCWD,
 						files[i]) < 0) {
-				if (errno == EINVAL &&
-					tc->ignore_mask & FAN_OPEN_EXEC) {
-					tst_res(TCONF,
-						"FAN_OPEN_EXEC not supported "
-						"in kernel?");
-					return -1;
-				} else if (errno == EINVAL) {
-					tst_brk(TCONF | TERRNO,
-						"CONFIG_FANOTIFY_ACCESS_"
-						"PERMISSIONS not configured in "
-						"kernel?");
-				} else {
-					tst_brk(TBROK | TERRNO,
-						"fanotify_mark (%d, "
-						"FAN_MARK_ADD | %s "
-						"| FAN_MARK_IGNORED_MASK, "
-						"%llx, AT_FDCWD, %s) failed",
-						fd_notify, mark->name,
-						tc->ignore_mask, files[i]);
-				}
+				tst_brk(TBROK | TERRNO,
+					"fanotify_mark (%d, "
+					"FAN_MARK_ADD | %s "
+					"| FAN_MARK_IGNORED_MASK, "
+					"%llx, AT_FDCWD, %s) failed",
+					fd_notify, mark->name,
+					tc->ignore_mask, files[i]);
 			}
 		}
 	}
@@ -249,6 +236,8 @@ cleanup:
 
 static void do_setup(void)
 {
+	support_exec_events = FANOTIFY_EXEC_EVENTS_SUPPORTED_BY_KERNEL(FAN_OPEN_EXEC);
+
 	sprintf(fname, "fname_%d", getpid());
 	SAFE_FILE_PRINTF(fname, "1");
 }
-- 
2.29.2

[LTP] [PATCH v3 2/5] fanotify: Handle supported features checks in setup()

[Cyril Hrubis]

Hi!
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify07.c b/testcases/kernel/syscalls/fanotify/fanotify07.c
> index c2e185710..f4e8ac9e6 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify07.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify07.c
> @@ -195,6 +195,8 @@ static void test_fanotify(void)
>  
>  static void setup(void)
>  {
> +	fanotify_access_permissions_supported_by_kernel();
> +
>  	sprintf(fname, "fname_%d", getpid());
>  	SAFE_FILE_PRINTF(fname, "%s", fname);
>  }

Shouldn't we drop the check for EINVAL in setup_instance() as well?

> diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
> index 90cf5cb5f..b95efb998 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify10.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
> @@ -64,6 +64,7 @@ static unsigned int fanotify_class[] = {
>  static int fd_notify[NUM_CLASSES][GROUPS_PER_PRIO];
>  
>  static char event_buf[EVENT_BUF_LEN];
> +static int support_exec_events;
>  
>  #define MOUNT_PATH "fs_mnt"
>  #define MNT2_PATH "mntpoint"
> @@ -451,6 +452,11 @@ static void test_fanotify(unsigned int n)
>  
>  	tst_res(TINFO, "Test #%d: %s", n, tc->tname);
>  
> +	if (support_exec_events != 0 && tc->expected_mask_with_ignore & FAN_OPEN_EXEC) {
> +		tst_res(TCONF | TERRNO, "FAN_OPEN_EXEC not supported in kernel?");
> +		return;
> +	}
> +

Maybe we should rename the variable to "exec_events_unsupported" then
we could write:

	if (exec_events_unsupported && tc->mask & FAM_OPEN_EXEC)

Which is a bit easier to understand.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3 2/5] fanotify: Handle supported features checks in setup()

[Petr Vorel]

Hi Cyril,

> > diff --git a/testcases/kernel/syscalls/fanotify/fanotify07.c b/testcases/kernel/syscalls/fanotify/fanotify07.c
> > index c2e185710..f4e8ac9e6 100644
> > --- a/testcases/kernel/syscalls/fanotify/fanotify07.c
> > +++ b/testcases/kernel/syscalls/fanotify/fanotify07.c
> > @@ -195,6 +195,8 @@ static void test_fanotify(void)

> >  static void setup(void)
> >  {
> > +	fanotify_access_permissions_supported_by_kernel();
> > +
> >  	sprintf(fname, "fname_%d", getpid());
> >  	SAFE_FILE_PRINTF(fname, "%s", fname);
> >  }

> Shouldn't we drop the check for EINVAL in setup_instance() as well?
I postponed that cleanup to next commit - whole part is being replaced by:

SAFE_FANOTIFY_MARK(fd, FAN_MARK_ADD, FAN_ACCESS_PERM, AT_FDCWD, fname);

https://patchwork.ozlabs.org/project/ltp/patch/20201113164944.26101-4-pvorel@suse.cz/

But if you find this confusing, I can remove CONFIG_FANOTIFY_ACCESS_PERMISSIONS
already in this commit.

> > diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
> > index 90cf5cb5f..b95efb998 100644
> > --- a/testcases/kernel/syscalls/fanotify/fanotify10.c
> > +++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
> > @@ -64,6 +64,7 @@ static unsigned int fanotify_class[] = {
> >  static int fd_notify[NUM_CLASSES][GROUPS_PER_PRIO];

> >  static char event_buf[EVENT_BUF_LEN];
> > +static int support_exec_events;

> >  #define MOUNT_PATH "fs_mnt"
> >  #define MNT2_PATH "mntpoint"
> > @@ -451,6 +452,11 @@ static void test_fanotify(unsigned int n)

> >  	tst_res(TINFO, "Test #%d: %s", n, tc->tname);

> > +	if (support_exec_events != 0 && tc->expected_mask_with_ignore & FAN_OPEN_EXEC) {
> > +		tst_res(TCONF | TERRNO, "FAN_OPEN_EXEC not supported in kernel?");
> > +		return;
> > +	}
> > +

> Maybe we should rename the variable to "exec_events_unsupported" then
> we could write:

> 	if (exec_events_unsupported && tc->mask & FAM_OPEN_EXEC)

> Which is a bit easier to understand.

+1.

Kind regards,
Petr

[LTP] [PATCH v3 3/5] fanotify: Introduce SAFE_FANOTIFY_MARK() macro

[Petr Vorel]

and function and use it in all tests which use fanotify_mark().

Motivation was not only to simplify the code but also check for
unsupported filesystems which was missing least for exfat on fanotify16,
because filesystem can be set via LTP_DEV_FS_TYPE environment variable
on tests which don't tests all filesystems.

Previous commit added check for FAN_ACCESS_PERM enablement (caused by
disabled CONFIG_FANOTIFY_ACCESS_PERMISSIONS) or FAN_OPEN_EXEC and
FAN_OPEN_EXEC_PERM support in kernel, which are tested in setup
functions for relevant tests, thus only FAN_MARK_FILESYSTEM is needed to
check in safe_fanotify_mark().

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/syscalls/fanotify/fanotify.h | 37 ++++++++++++-
 .../kernel/syscalls/fanotify/fanotify01.c     | 53 ++++---------------
 .../kernel/syscalls/fanotify/fanotify02.c     | 22 ++------
 .../kernel/syscalls/fanotify/fanotify03.c     | 18 +------
 .../kernel/syscalls/fanotify/fanotify04.c     | 32 +++--------
 .../kernel/syscalls/fanotify/fanotify05.c     |  9 +---
 .../kernel/syscalls/fanotify/fanotify06.c     | 21 ++------
 .../kernel/syscalls/fanotify/fanotify07.c     | 15 +-----
 .../kernel/syscalls/fanotify/fanotify09.c     | 19 ++-----
 .../kernel/syscalls/fanotify/fanotify10.c     | 36 ++-----------
 .../kernel/syscalls/fanotify/fanotify11.c     |  5 +-
 .../kernel/syscalls/fanotify/fanotify12.c     | 24 ++-------
 .../kernel/syscalls/fanotify/fanotify13.c     | 33 ++----------
 .../kernel/syscalls/fanotify/fanotify15.c     | 24 ++-------
 .../kernel/syscalls/fanotify/fanotify16.c     | 20 ++-----
 15 files changed, 90 insertions(+), 278 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 0c61f8ab7..18a2328cf 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -60,6 +60,7 @@ int safe_fanotify_init(const char *file, const int lineno,
 
 	return rval;
 }
+
 #define SAFE_FANOTIFY_INIT(fan, mode)  \
 	safe_fanotify_init(__FILE__, __LINE__, (fan), (mode))
 
@@ -189,6 +190,41 @@ struct fanotify_event_info_fid {
 #define MAX_HANDLE_SZ		128
 #endif
 
+static inline int safe_fanotify_mark(const char *file, const int lineno,
+			int fd, unsigned int flags, uint64_t mask,
+			int dfd, const char *pathname)
+{
+	int rval;
+
+	rval = fanotify_mark(fd, flags, mask, dfd, pathname);
+
+	if (rval == -1) {
+		if (errno == ENODEV || errno == EOPNOTSUPP || errno == EXDEV)
+			tst_brk(TCONF,
+				"%s:%d: FAN_REPORT_FID not supported on %s filesystem",
+				file, lineno, tst_device->fs_type);
+
+		if (errno == EINVAL && (flags & FAN_MARK_FILESYSTEM) == FAN_MARK_FILESYSTEM) {
+				tst_brk(TCONF,
+					"%s:%d: FAN_MARK_FILESYSTEM not supported by kernel?",
+					file, lineno);
+		}
+
+		tst_brk(TBROK | TERRNO, "%s:%d: fanotify_mark() failed",
+			file, lineno);
+	}
+
+	if (rval < -1) {
+		tst_brk(TBROK | TERRNO, "%s:%d: invalid fanotify_mark() return %d",
+			file, lineno, rval);
+	}
+
+	return rval;
+}
+
+#define SAFE_FANOTIFY_MARK(fd, flags, mask, dfd, pathname)  \
+	safe_fanotify_mark(__FILE__, __LINE__, (fd), (flags), (mask), (dfd), (pathname))
+
 /*
  * Helper function used to obtain fsid and file_handle for a given path.
  * Used by test files correlated to FAN_REPORT_FID functionality.
@@ -242,7 +278,6 @@ static inline void fanotify_save_fid(const char *path,
 #define INIT_FANOTIFY_MARK_TYPE(t) \
 	{ FAN_MARK_ ## t, "FAN_MARK_" #t }
 
-
 static inline void fanotify_access_permissions_supported_by_kernel(void)
 {
 	int fd;
diff --git a/testcases/kernel/syscalls/fanotify/fanotify01.c b/testcases/kernel/syscalls/fanotify/fanotify01.c
index 03e453f41..7690f6b88 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify01.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify01.c
@@ -101,19 +101,8 @@ static void test_fanotify(unsigned int n)
 			"failed", tc->init_flags);
 	}
 
-	if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
-			  FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
-			  AT_FDCWD, fname) < 0) {
-		if (errno == EINVAL && mark->flag == FAN_MARK_FILESYSTEM) {
-			tst_res(TCONF,
-				"FAN_MARK_FILESYSTEM not supported in kernel?");
-			return;
-		}
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_ADD, FAN_ACCESS | %s | "
-			"FAN_MODIFY | FAN_CLOSE | FAN_OPEN, AT_FDCWD, %s) "
-			"failed", fd_notify, mark->name, fname);
-	}
+	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag,
+			  FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN, AT_FDCWD, fname);
 
 	/*
 	 * generate sequence of events
@@ -161,14 +150,9 @@ static void test_fanotify(unsigned int n)
 	 */
 
 	/* Ignore access events */
-	if (fanotify_mark(fd_notify,
+	SAFE_FANOTIFY_MARK(fd_notify,
 			  FAN_MARK_ADD | mark->flag | FAN_MARK_IGNORED_MASK,
-			  FAN_ACCESS, AT_FDCWD, fname) < 0) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_ADD | %s | "
-			"FAN_MARK_IGNORED_MASK, FAN_ACCESS, AT_FDCWD, %s) "
-			"failed", fd_notify, mark->name, fname);
-	}
+			  FAN_ACCESS, AT_FDCWD, fname);
 
 	fd = SAFE_OPEN(fname, O_RDWR);
 	event_set[tst_count] = FAN_OPEN;
@@ -211,15 +195,9 @@ static void test_fanotify(unsigned int n)
 	 * Now ignore open & close events regardless of file
 	 * modifications
 	 */
-	if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag |
+	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag |
 			  FAN_MARK_IGNORED_MASK | FAN_MARK_IGNORED_SURV_MODIFY,
-			  FAN_OPEN | FAN_CLOSE, AT_FDCWD, fname) < 0) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_ADD | %s | "
-			"FAN_MARK_IGNORED_MASK | FAN_MARK_IGNORED_SURV_MODIFY, "
-			"FAN_OPEN | FAN_CLOSE, AT_FDCWD, %s) failed",
-			fd_notify, mark->name, fname);
-	}
+			  FAN_OPEN | FAN_CLOSE, AT_FDCWD, fname);
 
 	/* This event should be ignored */
 	fd = SAFE_OPEN(fname, O_RDWR);
@@ -240,15 +218,9 @@ static void test_fanotify(unsigned int n)
 	len += ret;
 
 	/* Now remove open and close from ignored mask */
-	if (fanotify_mark(fd_notify,
+	SAFE_FANOTIFY_MARK(fd_notify,
 			  FAN_MARK_REMOVE | mark->flag | FAN_MARK_IGNORED_MASK,
-			  FAN_OPEN | FAN_CLOSE, AT_FDCWD, fname) < 0) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_REMOVE | %s | "
-			"FAN_MARK_IGNORED_MASK, FAN_OPEN | FAN_CLOSE, "
-			"AT_FDCWD, %s) failed", fd_notify,
-			mark->name, fname);
-	}
+			  FAN_OPEN | FAN_CLOSE, AT_FDCWD, fname);
 
 	SAFE_CLOSE(fd);
 	event_set[tst_count] = FAN_CLOSE_WRITE;
@@ -344,14 +316,9 @@ pass:
 
 	}
 	/* Remove mark to clear FAN_MARK_IGNORED_SURV_MODIFY */
-	if (fanotify_mark(fd_notify, FAN_MARK_REMOVE | mark->flag,
+	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_REMOVE | mark->flag,
 			  FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
-			  AT_FDCWD, fname) < 0) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_REMOVE | %s, FAN_ACCESS | "
-			"FAN_MODIFY | FAN_CLOSE | FAN_OPEN, AT_FDCWD, %s) "
-			"failed", fd_notify, mark->name, fname);
-	}
+			  AT_FDCWD, fname);
 
 	SAFE_CLOSE(fd_notify);
 }
diff --git a/testcases/kernel/syscalls/fanotify/fanotify02.c b/testcases/kernel/syscalls/fanotify/fanotify02.c
index c578e0ae8..36c87648e 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify02.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify02.c
@@ -46,16 +46,9 @@ void test01(void)
 
 	int tst_count = 0;
 
-	if (fanotify_mark(fd_notify, FAN_MARK_ADD, FAN_ACCESS |
-			  FAN_MODIFY | FAN_CLOSE | FAN_OPEN |
-			  FAN_EVENT_ON_CHILD | FAN_ONDIR, AT_FDCWD,
-			  ".") < 0) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_ADD, FAN_ACCESS | "
-			"FAN_MODIFY | FAN_CLOSE | FAN_OPEN | "
-			"FAN_EVENT_ON_CHILD | FAN_ONDIR, AT_FDCWD, '.') "
-			"failed", fd_notify);
-	}
+	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD, FAN_ACCESS |
+			  FAN_MODIFY | FAN_CLOSE | FAN_OPEN | FAN_EVENT_ON_CHILD |
+			  FAN_ONDIR, AT_FDCWD, ".");
 
 	/*
 	 * generate sequence of events
@@ -102,13 +95,8 @@ void test01(void)
 	/*
 	 * now remove child mark
 	 */
-	if (fanotify_mark(fd_notify, FAN_MARK_REMOVE,
-			  FAN_EVENT_ON_CHILD, AT_FDCWD, ".") < 0) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK REMOVE, "
-			"FAN_EVENT_ON_CHILD, AT_FDCWD, '.') failed",
-			fd_notify);
-	}
+	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_REMOVE,
+			  FAN_EVENT_ON_CHILD, AT_FDCWD, ".");
 
 	/*
 	 * Do something to verify events didn't get generated
diff --git a/testcases/kernel/syscalls/fanotify/fanotify03.c b/testcases/kernel/syscalls/fanotify/fanotify03.c
index fbec652f6..e98694a66 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify03.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify03.c
@@ -221,22 +221,8 @@ static int setup_mark(unsigned int n)
 	fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
 
 	for (; i < ARRAY_SIZE(files); i++) {
-		if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
-				  tc->mask, AT_FDCWD, files[i]) < 0) {
-			if (errno == EINVAL &&
-					mark->flag == FAN_MARK_FILESYSTEM) {
-				tst_res(TCONF,
-					"FAN_MARK_FILESYSTEM not supported in "
-					"kernel?");
-				return -1;
-			} else {
-				tst_brk(TBROK | TERRNO,
-					"fanotify_mark(%d, FAN_MARK_ADD | %s, "
-					"FAN_ACCESS_PERM | FAN_OPEN_PERM, "
-					"AT_FDCWD, %s) failed.",
-					fd_notify, mark->name, fname);
-			}
-		}
+		SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag,
+				  tc->mask, AT_FDCWD, files[i]);
 	}
 
 	return 0;
diff --git a/testcases/kernel/syscalls/fanotify/fanotify04.c b/testcases/kernel/syscalls/fanotify/fanotify04.c
index 722ad5d41..a24e7f7c3 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify04.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify04.c
@@ -77,13 +77,8 @@ static void check_mark(char *file, unsigned long long flag, char *flagstr,
 		if (test_event)
 			test_event(file);
 
-		if (fanotify_mark(fd_notify, FAN_MARK_REMOVE | flag,
-				  FAN_OPEN, AT_FDCWD, file) < 0) {
-			tst_brk(TBROK | TERRNO,
-				"fanotify_mark (%d, FAN_MARK_REMOVE | %s, "
-				"FAN_OPEN, AT_FDCWD, '%s') failed",
-				fd_notify, flagstr, file);
-		}
+		SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_REMOVE | flag,
+				  FAN_OPEN, AT_FDCWD, file);
 	}
 }
 
@@ -191,29 +186,14 @@ void test01(void)
 	CHECK_MARK(sname, 0, 0, test_open_file);
 
 	/* Verify FAN_MARK_FLUSH destroys all inode marks */
-	if (fanotify_mark(fd_notify, FAN_MARK_ADD,
-			  FAN_OPEN, AT_FDCWD, fname) < 0) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_ADD, FAN_OPEN, "
-			"AT_FDCWD, '%s') failed", fd_notify, fname);
-	}
-	if (fanotify_mark(fd_notify, FAN_MARK_ADD,
-			  FAN_OPEN | FAN_ONDIR, AT_FDCWD, dir) < 0) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_ADD, FAN_OPEN | "
-			"FAN_ONDIR, AT_FDCWD, '%s') failed", fd_notify,
-			dir);
-	}
+	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD, FAN_OPEN, AT_FDCWD, fname);
+	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD, FAN_OPEN | FAN_ONDIR,
+			   AT_FDCWD, dir);
 	open_file(fname);
 	verify_event(S_IFREG);
 	open_dir(dir);
 	verify_event(S_IFDIR);
-	if (fanotify_mark(fd_notify, FAN_MARK_FLUSH,
-			  0, AT_FDCWD, ".") < 0) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_FLUSH, 0, "
-			"AT_FDCWD, '.') failed", fd_notify);
-	}
+	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_FLUSH, 0, AT_FDCWD, ".");
 
 	open_dir(dir);
 	verify_no_event();
diff --git a/testcases/kernel/syscalls/fanotify/fanotify05.c b/testcases/kernel/syscalls/fanotify/fanotify05.c
index e53cc333a..c13b9ad7b 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify05.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify05.c
@@ -107,13 +107,8 @@ static void setup(void)
 	fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF | FAN_NONBLOCK,
 			O_RDONLY);
 
-	if (fanotify_mark(fd_notify, FAN_MARK_MOUNT | FAN_MARK_ADD, FAN_OPEN,
-			  AT_FDCWD, MOUNT_PATH) < 0) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_MOUNT | FAN_MARK_ADD, "
-			"FAN_OPEN, AT_FDCWD, \".\") failed",
-			fd_notify);
-	}
+	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_MOUNT | FAN_MARK_ADD, FAN_OPEN,
+			  AT_FDCWD, MOUNT_PATH);
 }
 
 static void cleanup(void)
diff --git a/testcases/kernel/syscalls/fanotify/fanotify06.c b/testcases/kernel/syscalls/fanotify/fanotify06.c
index 99e312a4f..ac4901f6f 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify06.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify06.c
@@ -78,7 +78,6 @@ static struct tcase {
 static void create_fanotify_groups(void)
 {
 	unsigned int p, i;
-	int ret;
 
 	for (p = 0; p < FANOTIFY_PRIORITIES; p++) {
 		for (i = 0; i < GROUPS_PER_PRIO; i++) {
@@ -87,32 +86,20 @@ static void create_fanotify_groups(void)
 							     O_RDONLY);
 
 			/* Add mount mark for each group */
-			ret = fanotify_mark(fd_notify[p][i],
+			SAFE_FANOTIFY_MARK(fd_notify[p][i],
 					    FAN_MARK_ADD | FAN_MARK_MOUNT,
 					    FAN_MODIFY,
 					    AT_FDCWD, fname);
-			if (ret < 0) {
-				tst_brk(TBROK | TERRNO,
-					"fanotify_mark(%d, FAN_MARK_ADD | "
-					"FAN_MARK_MOUNT, FAN_MODIFY, AT_FDCWD,"
-					" %s) failed", fd_notify[p][i], fname);
-			}
+
 			/* Add ignore mark for groups with higher priority */
 			if (p == 0)
 				continue;
-			ret = fanotify_mark(fd_notify[p][i],
+
+			SAFE_FANOTIFY_MARK(fd_notify[p][i],
 					    FAN_MARK_ADD |
 					    FAN_MARK_IGNORED_MASK |
 					    FAN_MARK_IGNORED_SURV_MODIFY,
 					    FAN_MODIFY, AT_FDCWD, fname);
-			if (ret < 0) {
-				tst_brk(TBROK | TERRNO,
-					"fanotify_mark(%d, FAN_MARK_ADD | "
-					"FAN_MARK_IGNORED_MASK | "
-					"FAN_MARK_IGNORED_SURV_MODIFY, "
-					"FAN_MODIFY, AT_FDCWD, %s) failed",
-					fd_notify[p][i], fname);
-			}
 		}
 	}
 }
diff --git a/testcases/kernel/syscalls/fanotify/fanotify07.c b/testcases/kernel/syscalls/fanotify/fanotify07.c
index f4e8ac9e6..8b2c1c4c9 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify07.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify07.c
@@ -102,20 +102,7 @@ static int setup_instance(void)
 	int fd;
 
 	fd = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
-
-	if (fanotify_mark(fd, FAN_MARK_ADD, FAN_ACCESS_PERM, AT_FDCWD,
-			  fname) < 0) {
-		close(fd);
-		if (errno == EINVAL) {
-			tst_brk(TCONF | TERRNO,
-				"CONFIG_FANOTIFY_ACCESS_PERMISSIONS not "
-				"configured in kernel?");
-		} else {
-			tst_brk(TBROK | TERRNO,
-				"fanotify_mark (%d, FAN_MARK_ADD, FAN_ACCESS_PERM, "
-				"AT_FDCWD, %s) failed.", fd, fname);
-		}
-	}
+	SAFE_FANOTIFY_MARK(fd, FAN_MARK_ADD, FAN_ACCESS_PERM, AT_FDCWD, fname);
 
 	return fd;
 }
diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
index 83210bc1c..9c9938619 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify09.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
@@ -99,7 +99,6 @@ static void create_fanotify_groups(struct tcase *tc)
 {
 	struct fanotify_mark_type *mark = &tc->mark;
 	unsigned int i, onchild, ondir = tc->ondir;
-	int ret;
 
 	for (i = 0; i < NUM_GROUPS; i++) {
 		fd_notify[i] = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF |
@@ -111,17 +110,11 @@ static void create_fanotify_groups(struct tcase *tc)
 		 * but only the first group requests events on dir.
 		 */
 		onchild = (i == 0) ? FAN_EVENT_ON_CHILD | ondir : 0;
-		ret = fanotify_mark(fd_notify[i],
+		SAFE_FANOTIFY_MARK(fd_notify[i],
 				    FAN_MARK_ADD | mark->flag,
 				    FAN_CLOSE_NOWRITE | onchild,
 				    AT_FDCWD, tc->testdir);
-		if (ret < 0) {
-			tst_brk(TBROK | TERRNO,
-				"fanotify_mark(%d, FAN_MARK_ADD | %s, "
-				"%x, AT_FDCWD, '%s') failed",
-				fd_notify[i], mark->name,
-				FAN_CLOSE_NOWRITE | ondir, tc->testdir);
-		}
+
 		/*
 		 * Add inode mark on parent for each group with MODIFY event,
 		 * but only the first group requests events on child.
@@ -129,15 +122,9 @@ static void create_fanotify_groups(struct tcase *tc)
 		 * setting the DCACHE_FSNOTIFY_PARENT_WATCHED dentry
 		 * flag.
 		 */
-		ret = fanotify_mark(fd_notify[i], FAN_MARK_ADD,
+		SAFE_FANOTIFY_MARK(fd_notify[i], FAN_MARK_ADD,
 				    FAN_MODIFY | ondir | onchild,
 				    AT_FDCWD, ".");
-		if (ret < 0) {
-			tst_brk(TBROK | TERRNO,
-				"fanotify_mark(%d, FAN_MARK_ADD, "
-				"%x, AT_FDCWD, '.') failed",
-				fd_notify[i], FAN_MODIFY | ondir | onchild);
-		}
 	}
 }
 
diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
index b95efb998..ab906f3fb 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify10.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
@@ -288,7 +288,6 @@ static int create_fanotify_groups(unsigned int n)
 	struct fanotify_mark_type *mark, *ignore_mark;
 	unsigned int mark_ignored, mask;
 	unsigned int p, i;
-	int ret;
 
 	mark = &fanotify_mark_types[tc->mark_type];
 	ignore_mark = &fanotify_mark_types[tc->ignore_mark_type];
@@ -317,32 +316,12 @@ static int create_fanotify_groups(unsigned int n)
 			 * FAN_EVENT_ON_CHILD has no effect on filesystem/mount
 			 * or inode mark on non-directory.
 			 */
-			ret = fanotify_mark(fd_notify[p][i],
+			SAFE_FANOTIFY_MARK(fd_notify[p][i],
 					    FAN_MARK_ADD | mark->flag,
 					    tc->expected_mask_without_ignore |
 					    FAN_EVENT_ON_CHILD,
 					    AT_FDCWD, tc->mark_path);
-			if (ret < 0) {
-				if (errno == EINVAL &&
-				    tc->expected_mask_without_ignore &
-				    FAN_OPEN_EXEC) {
-					tst_res(TCONF,
-						"FAN_OPEN_EXEC not supported "
-						"by kernel?");
-					return -1;
-				} else if (errno == EINVAL &&
-					tc->mark_type == FANOTIFY_FILESYSTEM) {
-					tst_res(TCONF,
-						"FAN_MARK_FILESYSTEM not "
-						"supported in kernel?");
-					return -1;
-				}
-				tst_brk(TBROK | TERRNO,
-					"fanotify_mark(%d, FAN_MARK_ADD | %s,"
-					"FAN_OPEN, AT_FDCWD, %s) failed",
-					fd_notify[p][i], mark->name,
-					tc->mark_path);
-			}
+
 			/* Add ignore mark for groups with higher priority */
 			if (p == 0)
 				continue;
@@ -351,18 +330,9 @@ static int create_fanotify_groups(unsigned int n)
 			mark_ignored = FAN_MARK_IGNORED_MASK |
 					FAN_MARK_IGNORED_SURV_MODIFY;
 add_mark:
-			ret = fanotify_mark(fd_notify[p][i],
+			SAFE_FANOTIFY_MARK(fd_notify[p][i],
 					    FAN_MARK_ADD | ignore_mark->flag | mark_ignored,
 					    mask, AT_FDCWD, tc->ignore_path);
-			if (ret < 0) {
-				tst_brk(TBROK | TERRNO,
-					"fanotify_mark(%d, FAN_MARK_ADD | %s | %s, "
-					"%x, AT_FDCWD, %s) failed",
-					fd_notify[p][i], ignore_mark->name,
-					mark_ignored ? "FAN_MARK_IGNORED_MASK | "
-					"FAN_MARK_IGNORED_SURV_MODIFY" : "",
-					mask, tc->ignore_path);
-			}
 
 			/*
 			 * If ignored mask is on a parent watching children,
diff --git a/testcases/kernel/syscalls/fanotify/fanotify11.c b/testcases/kernel/syscalls/fanotify/fanotify11.c
index 9e8606c72..56b7153f8 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify11.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify11.c
@@ -55,7 +55,6 @@ static unsigned int tcases[] = {
 
 void test01(unsigned int i)
 {
-	int ret;
 	pthread_t p_id;
 	struct fanotify_event_metadata event;
 	int fd_notify;
@@ -76,10 +75,8 @@ void test01(unsigned int i)
 				tcases[i]);
 	}
 
-	ret = fanotify_mark(fd_notify, FAN_MARK_ADD,
+	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD,
 			FAN_ALL_EVENTS | FAN_EVENT_ON_CHILD, AT_FDCWD, ".");
-	if (ret != 0)
-		tst_brk(TBROK, "fanotify_mark FAN_MARK_ADD fail ret=%d", ret);
 
 	SAFE_PTHREAD_CREATE(&p_id, NULL, thread_create_file, NULL);
 
diff --git a/testcases/kernel/syscalls/fanotify/fanotify12.c b/testcases/kernel/syscalls/fanotify/fanotify12.c
index bddbdc37c..9adf8bc8a 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify12.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify12.c
@@ -146,31 +146,15 @@ static int setup_mark(unsigned int n)
 
 	for (; i < ARRAY_SIZE(files); i++) {
 		/* Setup normal mark on object */
-		if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
-					tc->mask, AT_FDCWD, files[i]) < 0) {
-			tst_brk(TBROK | TERRNO,
-				"fanotify_mark(%d, FAN_MARK_ADD | %s, "
-				"%llx, AT_FDCWD, %s) failed",
-				fd_notify,
-				mark->name,
-				tc->mask,
-				files[i]);
-		}
+		SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag,
+					tc->mask, AT_FDCWD, files[i]);
 
 		/* Setup ignore mark on object */
 		if (tc->ignore_mask) {
-			if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag
+			SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag
 						| FAN_MARK_IGNORED_MASK,
 						tc->ignore_mask, AT_FDCWD,
-						files[i]) < 0) {
-				tst_brk(TBROK | TERRNO,
-					"fanotify_mark (%d, "
-					"FAN_MARK_ADD | %s "
-					"| FAN_MARK_IGNORED_MASK, "
-					"%llx, AT_FDCWD, %s) failed",
-					fd_notify, mark->name,
-					tc->ignore_mask, files[i]);
-			}
+						files[i]);
 		}
 	}
 
diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
index c2a21bb66..997efaf08 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify13.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
@@ -116,28 +116,8 @@ static int setup_marks(unsigned int fd, struct test_case_t *tc)
 	struct fanotify_mark_type *mark = &tc->mark;
 
 	for (i = 0; i < ARRAY_SIZE(objects); i++) {
-		if (fanotify_mark(fd, FAN_MARK_ADD | mark->flag, tc->mask,
-					AT_FDCWD, objects[i].path) == -1) {
-			if (errno == EINVAL &&
-				mark->flag & FAN_MARK_FILESYSTEM) {
-				tst_res(TCONF,
-					"FAN_MARK_FILESYSTEM not supported by "
-					"kernel");
-				return 1;
-			} else if (errno == ENODEV &&
-				   !objects[i].fid.fsid.val[0] &&
-				   !objects[i].fid.fsid.val[1]) {
-				tst_res(TCONF,
-					"FAN_REPORT_FID not supported on "
-					"filesystem type %s",
-					tst_device->fs_type);
-				return 1;
-			}
-			tst_brk(TBROK | TERRNO,
-				"fanotify_mark(%d, FAN_MARK_ADD, FAN_OPEN, "
-				"AT_FDCWD, %s) failed",
-				fanotify_fd, objects[i].path);
-		}
+		SAFE_FANOTIFY_MARK(fd, FAN_MARK_ADD | mark->flag, tc->mask,
+					AT_FDCWD, objects[i].path);
 
 		/* Setup the expected mask for each generated event */
 		event_set[i].expected_mask = tc->mask;
@@ -292,13 +272,8 @@ static void do_setup(void)
 	 * uninitialized connector->fsid cache. This mark remains for all test
 	 * cases and is not expected to get any events (no writes in this test).
 	 */
-	if (fanotify_mark(nofid_fd, FAN_MARK_ADD, FAN_CLOSE_WRITE, AT_FDCWD,
-			  FILE_PATH_ONE) == -1) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark(%d, FAN_MARK_ADD, FAN_CLOSE_WRITE, "
-			"AT_FDCWD, "FILE_PATH_ONE") failed",
-			nofid_fd);
-	}
+	SAFE_FANOTIFY_MARK(nofid_fd, FAN_MARK_ADD, FAN_CLOSE_WRITE, AT_FDCWD,
+			  FILE_PATH_ONE);
 
 	/* Get the filesystem fsid and file handle for each created object */
 	get_object_stats();
diff --git a/testcases/kernel/syscalls/fanotify/fanotify15.c b/testcases/kernel/syscalls/fanotify/fanotify15.c
index d787a08e3..cf23df9e3 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify15.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify15.c
@@ -85,21 +85,10 @@ static void do_test(unsigned int number)
 
 	tst_res(TINFO, "Test #%d: %s", number, tc->tname);
 
-	if (fanotify_mark(fanotify_fd, FAN_MARK_ADD | mark->flag, tc->mask |
+	SAFE_FANOTIFY_MARK(fanotify_fd, FAN_MARK_ADD | mark->flag, tc->mask |
 				FAN_CREATE | FAN_DELETE | FAN_MOVE |
 				FAN_MODIFY | FAN_ONDIR,
-				AT_FDCWD, TEST_DIR) == -1) {
-		if (errno == ENODEV || errno == EOPNOTSUPP || errno == EXDEV)
-			tst_brk(TCONF,
-				"FAN_REPORT_FID not supported on %s "
-				"filesystem", tst_device->fs_type);
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark(%d, FAN_MARK_ADD | %s, "
-			"FAN_CREATE | FAN_DELETE | FAN_MOVE | "
-			"FAN_MODIFY | FAN_ONDIR | 0x%lx, "
-			"AT_FDCWD, %s) failed",
-			fanotify_fd, mark->name, tc->mask, TEST_DIR);
-	}
+				AT_FDCWD, TEST_DIR);
 
 	/* Save the test root dir fid */
 	fanotify_save_fid(TEST_DIR, &root_fid);
@@ -191,13 +180,8 @@ static void do_test(unsigned int number)
 	/*
 	 * Cleanup the mark
 	 */
-	if (fanotify_mark(fanotify_fd, FAN_MARK_FLUSH | mark->flag, 0,
-			  AT_FDCWD, TEST_DIR) < 0) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_FLUSH | %s, 0,"
-			"AT_FDCWD, '"TEST_DIR"') failed",
-			fanotify_fd, mark->name);
-	}
+	SAFE_FANOTIFY_MARK(fanotify_fd, FAN_MARK_FLUSH | mark->flag, 0,
+			  AT_FDCWD, TEST_DIR);
 
 	/* Process each event in buffer */
 	for (i = 0, metadata = (struct fanotify_event_metadata *) events_buf;
diff --git a/testcases/kernel/syscalls/fanotify/fanotify16.c b/testcases/kernel/syscalls/fanotify/fanotify16.c
index 7995a1688..6dbd7757c 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify16.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify16.c
@@ -174,13 +174,8 @@ static void do_test(unsigned int number)
 	/*
 	 * Watch dir modify events with name in filesystem/dir
 	 */
-	if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag, tc->mask,
-			  AT_FDCWD, MOUNT_PATH) < 0) {
-		tst_brk(TBROK | TERRNO,
-		    "fanotify_mark (%d, FAN_MARK_ADD | %s, 0x%lx, "
-		    "AT_FDCWD, '"MOUNT_PATH"') failed",
-		    fd_notify, mark->name, tc->mask);
-	}
+	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag, tc->mask,
+			   AT_FDCWD, MOUNT_PATH);
 
 	/* Save the mount root fid */
 	fanotify_save_fid(MOUNT_PATH, &root_fid);
@@ -195,14 +190,9 @@ static void do_test(unsigned int number)
 	/* Save the subdir fid */
 	fanotify_save_fid(dname1, &dir_fid);
 
-	if (tc->sub_mask &&
-	    fanotify_mark(fd_notify, FAN_MARK_ADD | sub_mark->flag, tc->sub_mask,
-			  AT_FDCWD, dname1) < 0) {
-		tst_brk(TBROK | TERRNO,
-		    "fanotify_mark (%d, FAN_MARK_ADD | %s, 0x%lx, "
-		    "AT_FDCWD, '%s') failed",
-		    fd_notify, sub_mark->name, tc->sub_mask, dname1);
-	}
+	if (tc->sub_mask)
+		SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | sub_mark->flag,
+				   tc->sub_mask, AT_FDCWD, dname1);
 
 	event_set[tst_count].mask = FAN_CREATE | FAN_ONDIR;
 	event_set[tst_count].fid = &root_fid;
-- 
2.29.2

[LTP] [PATCH v3 3/5] fanotify: Introduce SAFE_FANOTIFY_MARK() macro

[Cyril Hrubis]

Hi!
>  	return rval;
>  }
> +
>  #define SAFE_FANOTIFY_INIT(fan, mode)  \
>  	safe_fanotify_init(__FILE__, __LINE__, (fan), (mode))
>  
> @@ -189,6 +190,41 @@ struct fanotify_event_info_fid {
>  #define MAX_HANDLE_SZ		128
>  #endif
>  
> +static inline int safe_fanotify_mark(const char *file, const int lineno,
> +			int fd, unsigned int flags, uint64_t mask,
> +			int dfd, const char *pathname)
> +{
> +	int rval;
> +
> +	rval = fanotify_mark(fd, flags, mask, dfd, pathname);
> +
> +	if (rval == -1) {
> +		if (errno == ENODEV || errno == EOPNOTSUPP || errno == EXDEV)
> +			tst_brk(TCONF,
> +				"%s:%d: FAN_REPORT_FID not supported on %s filesystem",
> +				file, lineno, tst_device->fs_type);

I guess that we should use tst_brk_() and pass the file and lineno here
and in the rest of the tst_ calls in this function.

> +		if (errno == EINVAL && (flags & FAN_MARK_FILESYSTEM) == FAN_MARK_FILESYSTEM) {

The comparsion in (flags & FAN_MARK_FILESYSTEM) == FAN_MARK_FILESYSTEM
seems to be useless. How is this different from a simple
(flags & MARK_FILESYSTEM) when converted into a boolean value?

> +				tst_brk(TCONF,
> +					"%s:%d: FAN_MARK_FILESYSTEM not supported by kernel?",
> +					file, lineno);
> +		}
> +
> +		tst_brk(TBROK | TERRNO, "%s:%d: fanotify_mark() failed",
> +			file, lineno);
> +	}
> +
> +	if (rval < -1) {
> +		tst_brk(TBROK | TERRNO, "%s:%d: invalid fanotify_mark() return %d",
> +			file, lineno, rval);
> +	}
> +
> +	return rval;
> +}
> +
> +#define SAFE_FANOTIFY_MARK(fd, flags, mask, dfd, pathname)  \
> +	safe_fanotify_mark(__FILE__, __LINE__, (fd), (flags), (mask), (dfd), (pathname))
> +
>  /*
>   * Helper function used to obtain fsid and file_handle for a given path.
>   * Used by test files correlated to FAN_REPORT_FID functionality.
> @@ -242,7 +278,6 @@ static inline void fanotify_save_fid(const char *path,
>  #define INIT_FANOTIFY_MARK_TYPE(t) \
>  	{ FAN_MARK_ ## t, "FAN_MARK_" #t }
>  
> -
>  static inline void fanotify_access_permissions_supported_by_kernel(void)
>  {
>  	int fd;
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify01.c b/testcases/kernel/syscalls/fanotify/fanotify01.c
> index 03e453f41..7690f6b88 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify01.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify01.c
> @@ -101,19 +101,8 @@ static void test_fanotify(unsigned int n)
>  			"failed", tc->init_flags);
>  	}
>  
> -	if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
> -			  FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
> -			  AT_FDCWD, fname) < 0) {
> -		if (errno == EINVAL && mark->flag == FAN_MARK_FILESYSTEM) {
> -			tst_res(TCONF,
> -				"FAN_MARK_FILESYSTEM not supported in kernel?");
> -			return;
> -		}

Here we had tst_res(TCONF, ...) followed by a return but we will can
tst_brk() after the change. I guess that we may skip part of the test on
older kernels with that change.

> -		tst_brk(TBROK | TERRNO,
> -			"fanotify_mark (%d, FAN_MARK_ADD, FAN_ACCESS | %s | "
> -			"FAN_MODIFY | FAN_CLOSE | FAN_OPEN, AT_FDCWD, %s) "
> -			"failed", fd_notify, mark->name, fname);
> -	}
> +	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag,
> +			  FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN, AT_FDCWD, fname);
>  
>  	/*
>  	 * generate sequence of events
> @@ -161,14 +150,9 @@ static void test_fanotify(unsigned int n)
>  	 */
>  
>  	/* Ignore access events */
> -	if (fanotify_mark(fd_notify,
> +	SAFE_FANOTIFY_MARK(fd_notify,
>  			  FAN_MARK_ADD | mark->flag | FAN_MARK_IGNORED_MASK,
> -			  FAN_ACCESS, AT_FDCWD, fname) < 0) {
> -		tst_brk(TBROK | TERRNO,
> -			"fanotify_mark (%d, FAN_MARK_ADD | %s | "
> -			"FAN_MARK_IGNORED_MASK, FAN_ACCESS, AT_FDCWD, %s) "
> -			"failed", fd_notify, mark->name, fname);
> -	}
> +			  FAN_ACCESS, AT_FDCWD, fname);
>  
>  	fd = SAFE_OPEN(fname, O_RDWR);
>  	event_set[tst_count] = FAN_OPEN;
> @@ -211,15 +195,9 @@ static void test_fanotify(unsigned int n)
>  	 * Now ignore open & close events regardless of file
>  	 * modifications
>  	 */
> -	if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag |
> +	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag |
>  			  FAN_MARK_IGNORED_MASK | FAN_MARK_IGNORED_SURV_MODIFY,
> -			  FAN_OPEN | FAN_CLOSE, AT_FDCWD, fname) < 0) {
> -		tst_brk(TBROK | TERRNO,
> -			"fanotify_mark (%d, FAN_MARK_ADD | %s | "
> -			"FAN_MARK_IGNORED_MASK | FAN_MARK_IGNORED_SURV_MODIFY, "
> -			"FAN_OPEN | FAN_CLOSE, AT_FDCWD, %s) failed",
> -			fd_notify, mark->name, fname);
> -	}
> +			  FAN_OPEN | FAN_CLOSE, AT_FDCWD, fname);
>  
>  	/* This event should be ignored */
>  	fd = SAFE_OPEN(fname, O_RDWR);
> @@ -240,15 +218,9 @@ static void test_fanotify(unsigned int n)
>  	len += ret;
>  
>  	/* Now remove open and close from ignored mask */
> -	if (fanotify_mark(fd_notify,
> +	SAFE_FANOTIFY_MARK(fd_notify,
>  			  FAN_MARK_REMOVE | mark->flag | FAN_MARK_IGNORED_MASK,
> -			  FAN_OPEN | FAN_CLOSE, AT_FDCWD, fname) < 0) {
> -		tst_brk(TBROK | TERRNO,
> -			"fanotify_mark (%d, FAN_MARK_REMOVE | %s | "
> -			"FAN_MARK_IGNORED_MASK, FAN_OPEN | FAN_CLOSE, "
> -			"AT_FDCWD, %s) failed", fd_notify,
> -			mark->name, fname);
> -	}
> +			  FAN_OPEN | FAN_CLOSE, AT_FDCWD, fname);
>  
>  	SAFE_CLOSE(fd);
>  	event_set[tst_count] = FAN_CLOSE_WRITE;
> @@ -344,14 +316,9 @@ pass:
>  
>  	}
>  	/* Remove mark to clear FAN_MARK_IGNORED_SURV_MODIFY */
> -	if (fanotify_mark(fd_notify, FAN_MARK_REMOVE | mark->flag,
> +	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_REMOVE | mark->flag,
>  			  FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
> -			  AT_FDCWD, fname) < 0) {
> -		tst_brk(TBROK | TERRNO,
> -			"fanotify_mark (%d, FAN_MARK_REMOVE | %s, FAN_ACCESS | "
> -			"FAN_MODIFY | FAN_CLOSE | FAN_OPEN, AT_FDCWD, %s) "
> -			"failed", fd_notify, mark->name, fname);
> -	}
> +			  AT_FDCWD, fname);
>  
>  	SAFE_CLOSE(fd_notify);
>  }
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify02.c b/testcases/kernel/syscalls/fanotify/fanotify02.c
> index c578e0ae8..36c87648e 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify02.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify02.c
> @@ -46,16 +46,9 @@ void test01(void)
>  
>  	int tst_count = 0;
>  
> -	if (fanotify_mark(fd_notify, FAN_MARK_ADD, FAN_ACCESS |
> -			  FAN_MODIFY | FAN_CLOSE | FAN_OPEN |
> -			  FAN_EVENT_ON_CHILD | FAN_ONDIR, AT_FDCWD,
> -			  ".") < 0) {
> -		tst_brk(TBROK | TERRNO,
> -			"fanotify_mark (%d, FAN_MARK_ADD, FAN_ACCESS | "
> -			"FAN_MODIFY | FAN_CLOSE | FAN_OPEN | "
> -			"FAN_EVENT_ON_CHILD | FAN_ONDIR, AT_FDCWD, '.') "
> -			"failed", fd_notify);
> -	}
> +	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD, FAN_ACCESS |
> +			  FAN_MODIFY | FAN_CLOSE | FAN_OPEN | FAN_EVENT_ON_CHILD |
> +			  FAN_ONDIR, AT_FDCWD, ".");
>  
>  	/*
>  	 * generate sequence of events
> @@ -102,13 +95,8 @@ void test01(void)
>  	/*
>  	 * now remove child mark
>  	 */
> -	if (fanotify_mark(fd_notify, FAN_MARK_REMOVE,
> -			  FAN_EVENT_ON_CHILD, AT_FDCWD, ".") < 0) {
> -		tst_brk(TBROK | TERRNO,
> -			"fanotify_mark (%d, FAN_MARK REMOVE, "
> -			"FAN_EVENT_ON_CHILD, AT_FDCWD, '.') failed",
> -			fd_notify);
> -	}
> +	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_REMOVE,
> +			  FAN_EVENT_ON_CHILD, AT_FDCWD, ".");
>  
>  	/*
>  	 * Do something to verify events didn't get generated
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify03.c b/testcases/kernel/syscalls/fanotify/fanotify03.c
> index fbec652f6..e98694a66 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify03.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify03.c
> @@ -221,22 +221,8 @@ static int setup_mark(unsigned int n)
>  	fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
>  
>  	for (; i < ARRAY_SIZE(files); i++) {
> -		if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
> -				  tc->mask, AT_FDCWD, files[i]) < 0) {
> -			if (errno == EINVAL &&
> -					mark->flag == FAN_MARK_FILESYSTEM) {
> -				tst_res(TCONF,
> -					"FAN_MARK_FILESYSTEM not supported in "
> -					"kernel?");
> -				return -1;


Here as well.


> -			} else {
> -				tst_brk(TBROK | TERRNO,
> -					"fanotify_mark(%d, FAN_MARK_ADD | %s, "
> -					"FAN_ACCESS_PERM | FAN_OPEN_PERM, "
> -					"AT_FDCWD, %s) failed.",
> -					fd_notify, mark->name, fname);
> -			}
> -		}
> +		SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag,
> +				  tc->mask, AT_FDCWD, files[i]);
>  	}
>  
>  	return 0;
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify04.c b/testcases/kernel/syscalls/fanotify/fanotify04.c
> index 722ad5d41..a24e7f7c3 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify04.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify04.c
> @@ -77,13 +77,8 @@ static void check_mark(char *file, unsigned long long flag, char *flagstr,
>  		if (test_event)
>  			test_event(file);
>  
> -		if (fanotify_mark(fd_notify, FAN_MARK_REMOVE | flag,
> -				  FAN_OPEN, AT_FDCWD, file) < 0) {
> -			tst_brk(TBROK | TERRNO,
> -				"fanotify_mark (%d, FAN_MARK_REMOVE | %s, "
> -				"FAN_OPEN, AT_FDCWD, '%s') failed",
> -				fd_notify, flagstr, file);
> -		}
> +		SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_REMOVE | flag,
> +				  FAN_OPEN, AT_FDCWD, file);
>  	}
>  }
>  
> @@ -191,29 +186,14 @@ void test01(void)
>  	CHECK_MARK(sname, 0, 0, test_open_file);
>  
>  	/* Verify FAN_MARK_FLUSH destroys all inode marks */
> -	if (fanotify_mark(fd_notify, FAN_MARK_ADD,
> -			  FAN_OPEN, AT_FDCWD, fname) < 0) {
> -		tst_brk(TBROK | TERRNO,
> -			"fanotify_mark (%d, FAN_MARK_ADD, FAN_OPEN, "
> -			"AT_FDCWD, '%s') failed", fd_notify, fname);
> -	}
> -	if (fanotify_mark(fd_notify, FAN_MARK_ADD,
> -			  FAN_OPEN | FAN_ONDIR, AT_FDCWD, dir) < 0) {
> -		tst_brk(TBROK | TERRNO,
> -			"fanotify_mark (%d, FAN_MARK_ADD, FAN_OPEN | "
> -			"FAN_ONDIR, AT_FDCWD, '%s') failed", fd_notify,
> -			dir);
> -	}
> +	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD, FAN_OPEN, AT_FDCWD, fname);
> +	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD, FAN_OPEN | FAN_ONDIR,
> +			   AT_FDCWD, dir);
>  	open_file(fname);
>  	verify_event(S_IFREG);
>  	open_dir(dir);
>  	verify_event(S_IFDIR);
> -	if (fanotify_mark(fd_notify, FAN_MARK_FLUSH,
> -			  0, AT_FDCWD, ".") < 0) {
> -		tst_brk(TBROK | TERRNO,
> -			"fanotify_mark (%d, FAN_MARK_FLUSH, 0, "
> -			"AT_FDCWD, '.') failed", fd_notify);
> -	}
> +	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_FLUSH, 0, AT_FDCWD, ".");
>  
>  	open_dir(dir);
>  	verify_no_event();
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify05.c b/testcases/kernel/syscalls/fanotify/fanotify05.c
> index e53cc333a..c13b9ad7b 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify05.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify05.c
> @@ -107,13 +107,8 @@ static void setup(void)
>  	fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF | FAN_NONBLOCK,
>  			O_RDONLY);
>  
> -	if (fanotify_mark(fd_notify, FAN_MARK_MOUNT | FAN_MARK_ADD, FAN_OPEN,
> -			  AT_FDCWD, MOUNT_PATH) < 0) {
> -		tst_brk(TBROK | TERRNO,
> -			"fanotify_mark (%d, FAN_MARK_MOUNT | FAN_MARK_ADD, "
> -			"FAN_OPEN, AT_FDCWD, \".\") failed",
> -			fd_notify);
> -	}
> +	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_MOUNT | FAN_MARK_ADD, FAN_OPEN,
> +			  AT_FDCWD, MOUNT_PATH);
>  }
>  
>  static void cleanup(void)
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify06.c b/testcases/kernel/syscalls/fanotify/fanotify06.c
> index 99e312a4f..ac4901f6f 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify06.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify06.c
> @@ -78,7 +78,6 @@ static struct tcase {
>  static void create_fanotify_groups(void)
>  {
>  	unsigned int p, i;
> -	int ret;
>  
>  	for (p = 0; p < FANOTIFY_PRIORITIES; p++) {
>  		for (i = 0; i < GROUPS_PER_PRIO; i++) {
> @@ -87,32 +86,20 @@ static void create_fanotify_groups(void)
>  							     O_RDONLY);
>  
>  			/* Add mount mark for each group */
> -			ret = fanotify_mark(fd_notify[p][i],
> +			SAFE_FANOTIFY_MARK(fd_notify[p][i],
>  					    FAN_MARK_ADD | FAN_MARK_MOUNT,
>  					    FAN_MODIFY,
>  					    AT_FDCWD, fname);
> -			if (ret < 0) {
> -				tst_brk(TBROK | TERRNO,
> -					"fanotify_mark(%d, FAN_MARK_ADD | "
> -					"FAN_MARK_MOUNT, FAN_MODIFY, AT_FDCWD,"
> -					" %s) failed", fd_notify[p][i], fname);
> -			}
> +
>  			/* Add ignore mark for groups with higher priority */
>  			if (p == 0)
>  				continue;
> -			ret = fanotify_mark(fd_notify[p][i],
> +
> +			SAFE_FANOTIFY_MARK(fd_notify[p][i],
>  					    FAN_MARK_ADD |
>  					    FAN_MARK_IGNORED_MASK |
>  					    FAN_MARK_IGNORED_SURV_MODIFY,
>  					    FAN_MODIFY, AT_FDCWD, fname);
> -			if (ret < 0) {
> -				tst_brk(TBROK | TERRNO,
> -					"fanotify_mark(%d, FAN_MARK_ADD | "
> -					"FAN_MARK_IGNORED_MASK | "
> -					"FAN_MARK_IGNORED_SURV_MODIFY, "
> -					"FAN_MODIFY, AT_FDCWD, %s) failed",
> -					fd_notify[p][i], fname);
> -			}
>  		}
>  	}
>  }
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify07.c b/testcases/kernel/syscalls/fanotify/fanotify07.c
> index f4e8ac9e6..8b2c1c4c9 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify07.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify07.c
> @@ -102,20 +102,7 @@ static int setup_instance(void)
>  	int fd;
>  
>  	fd = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
> -
> -	if (fanotify_mark(fd, FAN_MARK_ADD, FAN_ACCESS_PERM, AT_FDCWD,
> -			  fname) < 0) {
> -		close(fd);
> -		if (errno == EINVAL) {
> -			tst_brk(TCONF | TERRNO,
> -				"CONFIG_FANOTIFY_ACCESS_PERMISSIONS not "
> -				"configured in kernel?");
> -		} else {
> -			tst_brk(TBROK | TERRNO,
> -				"fanotify_mark (%d, FAN_MARK_ADD, FAN_ACCESS_PERM, "
> -				"AT_FDCWD, %s) failed.", fd, fname);
> -		}
> -	}
> +	SAFE_FANOTIFY_MARK(fd, FAN_MARK_ADD, FAN_ACCESS_PERM, AT_FDCWD, fname);
>  
>  	return fd;
>  }
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
> index 83210bc1c..9c9938619 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify09.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
> @@ -99,7 +99,6 @@ static void create_fanotify_groups(struct tcase *tc)
>  {
>  	struct fanotify_mark_type *mark = &tc->mark;
>  	unsigned int i, onchild, ondir = tc->ondir;
> -	int ret;
>  
>  	for (i = 0; i < NUM_GROUPS; i++) {
>  		fd_notify[i] = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF |
> @@ -111,17 +110,11 @@ static void create_fanotify_groups(struct tcase *tc)
>  		 * but only the first group requests events on dir.
>  		 */
>  		onchild = (i == 0) ? FAN_EVENT_ON_CHILD | ondir : 0;
> -		ret = fanotify_mark(fd_notify[i],
> +		SAFE_FANOTIFY_MARK(fd_notify[i],
>  				    FAN_MARK_ADD | mark->flag,
>  				    FAN_CLOSE_NOWRITE | onchild,
>  				    AT_FDCWD, tc->testdir);
> -		if (ret < 0) {
> -			tst_brk(TBROK | TERRNO,
> -				"fanotify_mark(%d, FAN_MARK_ADD | %s, "
> -				"%x, AT_FDCWD, '%s') failed",
> -				fd_notify[i], mark->name,
> -				FAN_CLOSE_NOWRITE | ondir, tc->testdir);
> -		}
> +
>  		/*
>  		 * Add inode mark on parent for each group with MODIFY event,
>  		 * but only the first group requests events on child.
> @@ -129,15 +122,9 @@ static void create_fanotify_groups(struct tcase *tc)
>  		 * setting the DCACHE_FSNOTIFY_PARENT_WATCHED dentry
>  		 * flag.
>  		 */
> -		ret = fanotify_mark(fd_notify[i], FAN_MARK_ADD,
> +		SAFE_FANOTIFY_MARK(fd_notify[i], FAN_MARK_ADD,
>  				    FAN_MODIFY | ondir | onchild,
>  				    AT_FDCWD, ".");
> -		if (ret < 0) {
> -			tst_brk(TBROK | TERRNO,
> -				"fanotify_mark(%d, FAN_MARK_ADD, "
> -				"%x, AT_FDCWD, '.') failed",
> -				fd_notify[i], FAN_MODIFY | ondir | onchild);
> -		}
>  	}
>  }
>  
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
> index b95efb998..ab906f3fb 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify10.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
> @@ -288,7 +288,6 @@ static int create_fanotify_groups(unsigned int n)
>  	struct fanotify_mark_type *mark, *ignore_mark;
>  	unsigned int mark_ignored, mask;
>  	unsigned int p, i;
> -	int ret;
>  
>  	mark = &fanotify_mark_types[tc->mark_type];
>  	ignore_mark = &fanotify_mark_types[tc->ignore_mark_type];
> @@ -317,32 +316,12 @@ static int create_fanotify_groups(unsigned int n)
>  			 * FAN_EVENT_ON_CHILD has no effect on filesystem/mount
>  			 * or inode mark on non-directory.
>  			 */
> -			ret = fanotify_mark(fd_notify[p][i],
> +			SAFE_FANOTIFY_MARK(fd_notify[p][i],
>  					    FAN_MARK_ADD | mark->flag,
>  					    tc->expected_mask_without_ignore |
>  					    FAN_EVENT_ON_CHILD,
>  					    AT_FDCWD, tc->mark_path);
> -			if (ret < 0) {
> -				if (errno == EINVAL &&
> -				    tc->expected_mask_without_ignore &
> -				    FAN_OPEN_EXEC) {
> -					tst_res(TCONF,
> -						"FAN_OPEN_EXEC not supported "
> -						"by kernel?");
> -					return -1;
> -				} else if (errno == EINVAL &&
> -					tc->mark_type == FANOTIFY_FILESYSTEM) {
> -					tst_res(TCONF,
> -						"FAN_MARK_FILESYSTEM not "
> -						"supported in kernel?");
> -					return -1;
> -				}
> -				tst_brk(TBROK | TERRNO,
> -					"fanotify_mark(%d, FAN_MARK_ADD | %s,"
> -					"FAN_OPEN, AT_FDCWD, %s) failed",
> -					fd_notify[p][i], mark->name,
> -					tc->mark_path);
> -			}
> +
>  			/* Add ignore mark for groups with higher priority */
>  			if (p == 0)
>  				continue;
> @@ -351,18 +330,9 @@ static int create_fanotify_groups(unsigned int n)
>  			mark_ignored = FAN_MARK_IGNORED_MASK |
>  					FAN_MARK_IGNORED_SURV_MODIFY;
>  add_mark:
> -			ret = fanotify_mark(fd_notify[p][i],
> +			SAFE_FANOTIFY_MARK(fd_notify[p][i],
>  					    FAN_MARK_ADD | ignore_mark->flag | mark_ignored,
>  					    mask, AT_FDCWD, tc->ignore_path);
> -			if (ret < 0) {
> -				tst_brk(TBROK | TERRNO,
> -					"fanotify_mark(%d, FAN_MARK_ADD | %s | %s, "
> -					"%x, AT_FDCWD, %s) failed",
> -					fd_notify[p][i], ignore_mark->name,
> -					mark_ignored ? "FAN_MARK_IGNORED_MASK | "
> -					"FAN_MARK_IGNORED_SURV_MODIFY" : "",
> -					mask, tc->ignore_path);
> -			}
>  
>  			/*
>  			 * If ignored mask is on a parent watching children,
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify11.c b/testcases/kernel/syscalls/fanotify/fanotify11.c
> index 9e8606c72..56b7153f8 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify11.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify11.c
> @@ -55,7 +55,6 @@ static unsigned int tcases[] = {
>  
>  void test01(unsigned int i)
>  {
> -	int ret;
>  	pthread_t p_id;
>  	struct fanotify_event_metadata event;
>  	int fd_notify;
> @@ -76,10 +75,8 @@ void test01(unsigned int i)
>  				tcases[i]);
>  	}
>  
> -	ret = fanotify_mark(fd_notify, FAN_MARK_ADD,
> +	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD,
>  			FAN_ALL_EVENTS | FAN_EVENT_ON_CHILD, AT_FDCWD, ".");
> -	if (ret != 0)
> -		tst_brk(TBROK, "fanotify_mark FAN_MARK_ADD fail ret=%d", ret);
>  
>  	SAFE_PTHREAD_CREATE(&p_id, NULL, thread_create_file, NULL);
>  
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify12.c b/testcases/kernel/syscalls/fanotify/fanotify12.c
> index bddbdc37c..9adf8bc8a 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify12.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify12.c
> @@ -146,31 +146,15 @@ static int setup_mark(unsigned int n)
>  
>  	for (; i < ARRAY_SIZE(files); i++) {
>  		/* Setup normal mark on object */
> -		if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
> -					tc->mask, AT_FDCWD, files[i]) < 0) {
> -			tst_brk(TBROK | TERRNO,
> -				"fanotify_mark(%d, FAN_MARK_ADD | %s, "
> -				"%llx, AT_FDCWD, %s) failed",
> -				fd_notify,
> -				mark->name,
> -				tc->mask,
> -				files[i]);
> -		}
> +		SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag,
> +					tc->mask, AT_FDCWD, files[i]);
>  
>  		/* Setup ignore mark on object */
>  		if (tc->ignore_mask) {
> -			if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag
> +			SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag
>  						| FAN_MARK_IGNORED_MASK,
>  						tc->ignore_mask, AT_FDCWD,
> -						files[i]) < 0) {
> -				tst_brk(TBROK | TERRNO,
> -					"fanotify_mark (%d, "
> -					"FAN_MARK_ADD | %s "
> -					"| FAN_MARK_IGNORED_MASK, "
> -					"%llx, AT_FDCWD, %s) failed",
> -					fd_notify, mark->name,
> -					tc->ignore_mask, files[i]);
> -			}
> +						files[i]);
>  		}
>  	}
>  
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
> index c2a21bb66..997efaf08 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify13.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
> @@ -116,28 +116,8 @@ static int setup_marks(unsigned int fd, struct test_case_t *tc)
>  	struct fanotify_mark_type *mark = &tc->mark;
>  
>  	for (i = 0; i < ARRAY_SIZE(objects); i++) {
> -		if (fanotify_mark(fd, FAN_MARK_ADD | mark->flag, tc->mask,
> -					AT_FDCWD, objects[i].path) == -1) {
> -			if (errno == EINVAL &&
> -				mark->flag & FAN_MARK_FILESYSTEM) {
> -				tst_res(TCONF,
> -					"FAN_MARK_FILESYSTEM not supported by "
> -					"kernel");
> -				return 1;
> -			} else if (errno == ENODEV &&
> -				   !objects[i].fid.fsid.val[0] &&
> -				   !objects[i].fid.fsid.val[1]) {
> -				tst_res(TCONF,
> -					"FAN_REPORT_FID not supported on "
> -					"filesystem type %s",
> -					tst_device->fs_type);
> -				return 1;

And here as well.

> -			}
> -			tst_brk(TBROK | TERRNO,
> -				"fanotify_mark(%d, FAN_MARK_ADD, FAN_OPEN, "
> -				"AT_FDCWD, %s) failed",
> -				fanotify_fd, objects[i].path);
> -		}
> +		SAFE_FANOTIFY_MARK(fd, FAN_MARK_ADD | mark->flag, tc->mask,
> +					AT_FDCWD, objects[i].path);
>  
>  		/* Setup the expected mask for each generated event */
>  		event_set[i].expected_mask = tc->mask;

Otherwise it looks like a nice cleanup.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3 3/5] fanotify: Introduce SAFE_FANOTIFY_MARK() macro

[Amir Goldstein]

On Thu, Nov 19, 2020 at 12:26 PM Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> >       return rval;
> >  }
> > +
> >  #define SAFE_FANOTIFY_INIT(fan, mode)  \
> >       safe_fanotify_init(__FILE__, __LINE__, (fan), (mode))
> >
> > @@ -189,6 +190,41 @@ struct fanotify_event_info_fid {
> >  #define MAX_HANDLE_SZ                128
> >  #endif
> >
> > +static inline int safe_fanotify_mark(const char *file, const int lineno,
> > +                     int fd, unsigned int flags, uint64_t mask,
> > +                     int dfd, const char *pathname)
> > +{
> > +     int rval;
> > +
> > +     rval = fanotify_mark(fd, flags, mask, dfd, pathname);
> > +
> > +     if (rval == -1) {
> > +             if (errno == ENODEV || errno == EOPNOTSUPP || errno == EXDEV)
> > +                     tst_brk(TCONF,
> > +                             "%s:%d: FAN_REPORT_FID not supported on %s filesystem",
> > +                             file, lineno, tst_device->fs_type);
>
> I guess that we should use tst_brk_() and pass the file and lineno here
> and in the rest of the tst_ calls in this function.
>
> > +             if (errno == EINVAL && (flags & FAN_MARK_FILESYSTEM) == FAN_MARK_FILESYSTEM) {
>
> The comparsion in (flags & FAN_MARK_FILESYSTEM) == FAN_MARK_FILESYSTEM
> seems to be useless. How is this different from a simple
> (flags & MARK_FILESYSTEM) when converted into a boolean value?
>
> > +                             tst_brk(TCONF,
> > +                                     "%s:%d: FAN_MARK_FILESYSTEM not supported by kernel?",
> > +                                     file, lineno);
> > +             }
> > +
> > +             tst_brk(TBROK | TERRNO, "%s:%d: fanotify_mark() failed",
> > +                     file, lineno);
> > +     }
> > +
> > +     if (rval < -1) {
> > +             tst_brk(TBROK | TERRNO, "%s:%d: invalid fanotify_mark() return %d",
> > +                     file, lineno, rval);
> > +     }
> > +
> > +     return rval;
> > +}
> > +
> > +#define SAFE_FANOTIFY_MARK(fd, flags, mask, dfd, pathname)  \
> > +     safe_fanotify_mark(__FILE__, __LINE__, (fd), (flags), (mask), (dfd), (pathname))
> > +
> >  /*
> >   * Helper function used to obtain fsid and file_handle for a given path.
> >   * Used by test files correlated to FAN_REPORT_FID functionality.
> > @@ -242,7 +278,6 @@ static inline void fanotify_save_fid(const char *path,
> >  #define INIT_FANOTIFY_MARK_TYPE(t) \
> >       { FAN_MARK_ ## t, "FAN_MARK_" #t }
> >
> > -
> >  static inline void fanotify_access_permissions_supported_by_kernel(void)
> >  {
> >       int fd;
> > diff --git a/testcases/kernel/syscalls/fanotify/fanotify01.c b/testcases/kernel/syscalls/fanotify/fanotify01.c
> > index 03e453f41..7690f6b88 100644
> > --- a/testcases/kernel/syscalls/fanotify/fanotify01.c
> > +++ b/testcases/kernel/syscalls/fanotify/fanotify01.c
> > @@ -101,19 +101,8 @@ static void test_fanotify(unsigned int n)
> >                       "failed", tc->init_flags);
> >       }
> >
> > -     if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
> > -                       FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
> > -                       AT_FDCWD, fname) < 0) {
> > -             if (errno == EINVAL && mark->flag == FAN_MARK_FILESYSTEM) {
> > -                     tst_res(TCONF,
> > -                             "FAN_MARK_FILESYSTEM not supported in kernel?");
> > -                     return;
> > -             }
>
> Here we had tst_res(TCONF, ...) followed by a return but we will can
> tst_brk() after the change. I guess that we may skip part of the test on
> older kernels with that change.
>

That's not good. I missed that in my review.
There are many tests where only the FAN_MARK_FILESYSTEM
test cases are expected to result in TCONF, but the rest of the test
cases should run.

In most of these tests the FAN_MARK_FILESYSTEM test cases are
last because they were added later. This is not the case with fanotify01
and fanotify15 and we do not want to reply on the ordering anyway.

Thanks,
Amir.

[LTP] [PATCH v3 3/5] fanotify: Introduce SAFE_FANOTIFY_MARK() macro

[Petr Vorel]

Hi Amir,

> > > +++ b/testcases/kernel/syscalls/fanotify/fanotify01.c
> > > @@ -101,19 +101,8 @@ static void test_fanotify(unsigned int n)
> > >                       "failed", tc->init_flags);
> > >       }

> > > -     if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
> > > -                       FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
> > > -                       AT_FDCWD, fname) < 0) {
> > > -             if (errno == EINVAL && mark->flag == FAN_MARK_FILESYSTEM) {
> > > -                     tst_res(TCONF,
> > > -                             "FAN_MARK_FILESYSTEM not supported in kernel?");
> > > -                     return;
> > > -             }

> > Here we had tst_res(TCONF, ...) followed by a return but we will can
> > tst_brk() after the change. I guess that we may skip part of the test on
> > older kernels with that change.


> That's not good. I missed that in my review.
> There are many tests where only the FAN_MARK_FILESYSTEM
> test cases are expected to result in TCONF, but the rest of the test
> cases should run.

I'm not sure if I understand you. Is my approach correct here?

Kind regards,
Petr

> In most of these tests the FAN_MARK_FILESYSTEM test cases are
> last because they were added later. This is not the case with fanotify01
> and fanotify15 and we do not want to reply on the ordering anyway.

> Thanks,
> Amir.

[LTP] [PATCH v3 3/5] fanotify: Introduce SAFE_FANOTIFY_MARK() macro

[Amir Goldstein]

On Wed, Nov 25, 2020 at 4:16 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Amir,
>
> > > > +++ b/testcases/kernel/syscalls/fanotify/fanotify01.c
> > > > @@ -101,19 +101,8 @@ static void test_fanotify(unsigned int n)
> > > >                       "failed", tc->init_flags);
> > > >       }
>
> > > > -     if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
> > > > -                       FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
> > > > -                       AT_FDCWD, fname) < 0) {
> > > > -             if (errno == EINVAL && mark->flag == FAN_MARK_FILESYSTEM) {
> > > > -                     tst_res(TCONF,
> > > > -                             "FAN_MARK_FILESYSTEM not supported in kernel?");
> > > > -                     return;
> > > > -             }
>
> > > Here we had tst_res(TCONF, ...) followed by a return but we will can
> > > tst_brk() after the change. I guess that we may skip part of the test on
> > > older kernels with that change.
>
>
> > That's not good. I missed that in my review.
> > There are many tests where only the FAN_MARK_FILESYSTEM
> > test cases are expected to result in TCONF, but the rest of the test
> > cases should run.
>
> I'm not sure if I understand you. Is my approach correct here?
>

I think it is not correct, but please tell me what the outcome is when running
test fanotify01 on kernel < v5.1.

The expected outcome is that the 2 first test cases PASS and the rest of the
test cases are skipped (TCONF).

Now what happens if you reorder the 2 first test cases to be last in the tcases
array? Will they run@all or will tst_brk(TCONF, ... in safe_fanotify_mark()
cause the rest of the test cases not to run?

I'm just not sure how tst_brk() behaves with test cases, but I have a feeling
that it won't result in the expected outcome?

Thanks,
Amir.

[LTP] [PATCH v3 3/5] fanotify: Introduce SAFE_FANOTIFY_MARK() macro

[Petr Vorel]

Hi Amir,

> > > > +++ b/testcases/kernel/syscalls/fanotify/fanotify01.c
> > > > @@ -101,19 +101,8 @@ static void test_fanotify(unsigned int n)
> > > >                       "failed", tc->init_flags);
> > > >       }

> > > > -     if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
> > > > -                       FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
> > > > -                       AT_FDCWD, fname) < 0) {
> > > > -             if (errno == EINVAL && mark->flag == FAN_MARK_FILESYSTEM) {
> > > > -                     tst_res(TCONF,
> > > > -                             "FAN_MARK_FILESYSTEM not supported in kernel?");
> > > > -                     return;
> > > > -             }

> > > Here we had tst_res(TCONF, ...) followed by a return but we will can
> > > tst_brk() after the change. I guess that we may skip part of the test on
> > > older kernels with that change.


> > That's not good. I missed that in my review.
> > There are many tests where only the FAN_MARK_FILESYSTEM
> > test cases are expected to result in TCONF, but the rest of the test
> > cases should run.

> I'm not sure if I understand you. Is my approach correct here?
OK, I got that, I cannot use SAFE_FANOTIFY_MARK() in test_fanotify() in fanotify01.c
and in setup_marks() in fanotify13.c.

But FAN_REPORT_FID in is on both files already checked after fanotify_init()
call. Not sure if it must be check also for fanotify_mark(), because it's
only in FANOTIFY_INIT_FLAGS (via FANOTIFY_FID_BITS). FANOTIFY_MARK_FLAGS has
other flags.

If yes, I'll probably need to create fanotify_supported_by_kernel(...), which
check for all not supported flags and will be used in those 2 places and in
safe_fanotify_init(). Something like this:

typedef void (*tst_res_func_t)(const char *file, const int lineno,
		int ttype, const char *fmt, ...);

int fanotify_flags_supported_by_kernel(const char *file, const int lineno,
	unsigned int flags, int strict)
{
	tst_res_func_t res_func = tst_res_;
	int unsupported = 0;

	if (strict)
		res_func = tst_brk_;

	if (errno == EINVAL) {
		if (flags & FAN_REPORT_TID) {
			tst_res_(file, lineno, TINFO,
					 "FAN_REPORT_TID not supported by kernel?");
			unsupported = 1;
		}

		if (flags & FAN_REPORT_FID) {
			tst_res_(file, lineno, TINFO,
					 "FAN_REPORT_FID not supported by kernel?");
			unsupported = 1;
		}

		if (flags & FAN_REPORT_DIR_FID) {
			tst_res_(file, lineno, TINFO,
					 "FAN_REPORT_DIR_FID not supported by kernel?");
			unsupported = 1;
		}

		if (flags & FAN_REPORT_NAME) {
			tst_res_(file, lineno, TINFO,
					 "FAN_REPORT_NAME not supported by kernel?");
			unsupported = 1;
		}

		if (unsupported)
			res_func(file, lineno, TCONF, "Unsupported configuration, see above");
		else
			tst_brk_(file, lineno, TBROK, "Unknown failure");

		return -1;
	}

	return 0;
}

These are flags for fanotify_init(). Flags for fanotify_mark() are currently
handled by fanotify_exec_events_supported_by_kernel() (used for FAN_OPEN_EXEC
and FAN_OPEN_EXEC_PERM), using different approach. Testing fanotify_mark() flags
support in advance in setup makes tests faster, I'm just not happy we use
different approach. Any tip for improving this or improving readability is
welcome.

Kind regards,
Petr

> > In most of these tests the FAN_MARK_FILESYSTEM test cases are
> > last because they were added later. This is not the case with fanotify01
> > and fanotify15 and we do not want to reply on the ordering anyway.

> > Thanks,
> > Amir.

[LTP] [PATCH v3 3/5] fanotify: Introduce SAFE_FANOTIFY_MARK() macro

[Amir Goldstein]

On Wed, Nov 25, 2020 at 8:24 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Amir,
>
> > > > > +++ b/testcases/kernel/syscalls/fanotify/fanotify01.c
> > > > > @@ -101,19 +101,8 @@ static void test_fanotify(unsigned int n)
> > > > >                       "failed", tc->init_flags);
> > > > >       }
>
> > > > > -     if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
> > > > > -                       FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
> > > > > -                       AT_FDCWD, fname) < 0) {
> > > > > -             if (errno == EINVAL && mark->flag == FAN_MARK_FILESYSTEM) {
> > > > > -                     tst_res(TCONF,
> > > > > -                             "FAN_MARK_FILESYSTEM not supported in kernel?");
> > > > > -                     return;
> > > > > -             }
>
> > > > Here we had tst_res(TCONF, ...) followed by a return but we will can
> > > > tst_brk() after the change. I guess that we may skip part of the test on
> > > > older kernels with that change.
>
>
> > > That's not good. I missed that in my review.
> > > There are many tests where only the FAN_MARK_FILESYSTEM
> > > test cases are expected to result in TCONF, but the rest of the test
> > > cases should run.
>
> > I'm not sure if I understand you. Is my approach correct here?
> OK, I got that, I cannot use SAFE_FANOTIFY_MARK() in test_fanotify() in fanotify01.c
> and in setup_marks() in fanotify13.c.

I gave fanotify01 as an example.
There are many such cases, like fanotify03.
The point is we cannot replace tst_res() with tst_brk() when only some of the
test cases may be supported.

>
> But FAN_REPORT_FID in is on both files already checked after fanotify_init()
> call. Not sure if it must be check also for fanotify_mark(), because it's
> only in FANOTIFY_INIT_FLAGS (via FANOTIFY_FID_BITS). FANOTIFY_MARK_FLAGS has
> other flags.
>
> If yes, I'll probably need to create fanotify_supported_by_kernel(...), which
> check for all not supported flags and will be used in those 2 places and in
> safe_fanotify_init(). Something like this:
>
> typedef void (*tst_res_func_t)(const char *file, const int lineno,
>                 int ttype, const char *fmt, ...);
>
> int fanotify_flags_supported_by_kernel(const char *file, const int lineno,
>         unsigned int flags, int strict)
> {
>         tst_res_func_t res_func = tst_res_;
>         int unsupported = 0;
>
>         if (strict)
>                 res_func = tst_brk_;
>
>         if (errno == EINVAL) {
>                 if (flags & FAN_REPORT_TID) {
>                         tst_res_(file, lineno, TINFO,
>                                          "FAN_REPORT_TID not supported by kernel?");
>                         unsupported = 1;
>                 }
>
>                 if (flags & FAN_REPORT_FID) {
>                         tst_res_(file, lineno, TINFO,
>                                          "FAN_REPORT_FID not supported by kernel?");
>                         unsupported = 1;
>                 }
>
>                 if (flags & FAN_REPORT_DIR_FID) {
>                         tst_res_(file, lineno, TINFO,
>                                          "FAN_REPORT_DIR_FID not supported by kernel?");
>                         unsupported = 1;
>                 }
>
>                 if (flags & FAN_REPORT_NAME) {
>                         tst_res_(file, lineno, TINFO,
>                                          "FAN_REPORT_NAME not supported by kernel?");
>                         unsupported = 1;
>                 }
>
>                 if (unsupported)
>                         res_func(file, lineno, TCONF, "Unsupported configuration, see above");
>                 else
>                         tst_brk_(file, lineno, TBROK, "Unknown failure");
>
>                 return -1;
>         }
>
>         return 0;
> }
>

That seems too much and adds more noise than valuable info in many cases
or maybe I didn't understand.

> These are flags for fanotify_init(). Flags for fanotify_mark() are currently
> handled by fanotify_exec_events_supported_by_kernel() (used for FAN_OPEN_EXEC
> and FAN_OPEN_EXEC_PERM), using different approach. Testing fanotify_mark() flags
> support in advance in setup makes tests faster, I'm just not happy we use
> different approach. Any tip for improving this or improving readability is
> welcome.
>

I think the best would be to always test in advance like exec events,
for FAN_REPORT_ fanotify_init() flags and FAN_MARK_FILESYSTEM
fanotify_mark() flag whenever relevant.

I didn't go over all tests to see how that would look, but I have a feeling
that would end up being the cleanest approach.

Thanks,
Amir.

[LTP] [PATCH v3 3/5] fanotify: Introduce SAFE_FANOTIFY_MARK() macro

[Petr Vorel]

Hi Amir,

> On Wed, Nov 25, 2020 at 8:24 PM Petr Vorel <pvorel@suse.cz> wrote:

> > Hi Amir,

> > > > > > +++ b/testcases/kernel/syscalls/fanotify/fanotify01.c
> > > > > > @@ -101,19 +101,8 @@ static void test_fanotify(unsigned int n)
> > > > > >                       "failed", tc->init_flags);
> > > > > >       }

> > > > > > -     if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
> > > > > > -                       FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
> > > > > > -                       AT_FDCWD, fname) < 0) {
> > > > > > -             if (errno == EINVAL && mark->flag == FAN_MARK_FILESYSTEM) {
> > > > > > -                     tst_res(TCONF,
> > > > > > -                             "FAN_MARK_FILESYSTEM not supported in kernel?");
> > > > > > -                     return;
> > > > > > -             }

> > > > > Here we had tst_res(TCONF, ...) followed by a return but we will can
> > > > > tst_brk() after the change. I guess that we may skip part of the test on
> > > > > older kernels with that change.


> > > > That's not good. I missed that in my review.
> > > > There are many tests where only the FAN_MARK_FILESYSTEM
> > > > test cases are expected to result in TCONF, but the rest of the test
> > > > cases should run.

> > > I'm not sure if I understand you. Is my approach correct here?
> > OK, I got that, I cannot use SAFE_FANOTIFY_MARK() in test_fanotify() in fanotify01.c
> > and in setup_marks() in fanotify13.c.

> I gave fanotify01 as an example.
> There are many such cases, like fanotify03.

> The point is we cannot replace tst_res() with tst_brk() when only some of the
> test cases may be supported.

Sure, I'll check in all tests that tst_res() won't be replaced with tst_brk().

> > But FAN_REPORT_FID in is on both files already checked after fanotify_init()
> > call. Not sure if it must be check also for fanotify_mark(), because it's
> > only in FANOTIFY_INIT_FLAGS (via FANOTIFY_FID_BITS). FANOTIFY_MARK_FLAGS has
> > other flags.

> > If yes, I'll probably need to create fanotify_supported_by_kernel(...), which
> > check for all not supported flags and will be used in those 2 places and in
> > safe_fanotify_init(). Something like this:

> > typedef void (*tst_res_func_t)(const char *file, const int lineno,
> >                 int ttype, const char *fmt, ...);

> > int fanotify_flags_supported_by_kernel(const char *file, const int lineno,
> >         unsigned int flags, int strict)
> > {
> >         tst_res_func_t res_func = tst_res_;
> >         int unsupported = 0;

> >         if (strict)
> >                 res_func = tst_brk_;

> >         if (errno == EINVAL) {
> >                 if (flags & FAN_REPORT_TID) {
> >                         tst_res_(file, lineno, TINFO,
> >                                          "FAN_REPORT_TID not supported by kernel?");
> >                         unsupported = 1;
> >                 }

> >                 if (flags & FAN_REPORT_FID) {
> >                         tst_res_(file, lineno, TINFO,
> >                                          "FAN_REPORT_FID not supported by kernel?");
> >                         unsupported = 1;
> >                 }

> >                 if (flags & FAN_REPORT_DIR_FID) {
> >                         tst_res_(file, lineno, TINFO,
> >                                          "FAN_REPORT_DIR_FID not supported by kernel?");
> >                         unsupported = 1;
> >                 }

> >                 if (flags & FAN_REPORT_NAME) {
> >                         tst_res_(file, lineno, TINFO,
> >                                          "FAN_REPORT_NAME not supported by kernel?");
> >                         unsupported = 1;
> >                 }

> >                 if (unsupported)
> >                         res_func(file, lineno, TCONF, "Unsupported configuration, see above");
> >                 else
> >                         tst_brk_(file, lineno, TBROK, "Unknown failure");

> >                 return -1;
> >         }

> >         return 0;
> > }


> That seems too much and adds more noise than valuable info in many cases
> or maybe I didn't understand.

> > These are flags for fanotify_init(). Flags for fanotify_mark() are currently
> > handled by fanotify_exec_events_supported_by_kernel() (used for FAN_OPEN_EXEC
> > and FAN_OPEN_EXEC_PERM), using different approach. Testing fanotify_mark() flags
> > support in advance in setup makes tests faster, I'm just not happy we use
> > different approach. Any tip for improving this or improving readability is
> > welcome.


> I think the best would be to always test in advance like exec events,
> for FAN_REPORT_ fanotify_init() flags and FAN_MARK_FILESYSTEM
> fanotify_mark() flag whenever relevant.

> I didn't go over all tests to see how that would look, but I have a feeling
> that would end up being the cleanest approach.

> Thanks,
> Amir.

OK, I'll have a look whether FAN_REPORT_* will be easy to transform to checks in
advance.

I'll also try to wrote some automatic detection for testcases which use
struct tcase (looping the struct and collect flags with & and pass it to some
function). Maybe too complicated having to declare what is required to check
is something which is IMHO error prone (we probably forget to update what is
needed to be checked when we add/remove/change test structs).

Kind regards,
Petr

[LTP] [PATCH v3 3/5] fanotify: Introduce SAFE_FANOTIFY_MARK() macro

[Amir Goldstein]

On Wed, Nov 25, 2020 at 10:25 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Amir,
>
> > On Wed, Nov 25, 2020 at 8:24 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> > > Hi Amir,
>
> > > > > > > +++ b/testcases/kernel/syscalls/fanotify/fanotify01.c
> > > > > > > @@ -101,19 +101,8 @@ static void test_fanotify(unsigned int n)
> > > > > > >                       "failed", tc->init_flags);
> > > > > > >       }
>
> > > > > > > -     if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
> > > > > > > -                       FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
> > > > > > > -                       AT_FDCWD, fname) < 0) {
> > > > > > > -             if (errno == EINVAL && mark->flag == FAN_MARK_FILESYSTEM) {
> > > > > > > -                     tst_res(TCONF,
> > > > > > > -                             "FAN_MARK_FILESYSTEM not supported in kernel?");
> > > > > > > -                     return;
> > > > > > > -             }
>
> > > > > > Here we had tst_res(TCONF, ...) followed by a return but we will can
> > > > > > tst_brk() after the change. I guess that we may skip part of the test on
> > > > > > older kernels with that change.
>
>
> > > > > That's not good. I missed that in my review.
> > > > > There are many tests where only the FAN_MARK_FILESYSTEM
> > > > > test cases are expected to result in TCONF, but the rest of the test
> > > > > cases should run.
>
> > > > I'm not sure if I understand you. Is my approach correct here?
> > > OK, I got that, I cannot use SAFE_FANOTIFY_MARK() in test_fanotify() in fanotify01.c
> > > and in setup_marks() in fanotify13.c.
>
> > I gave fanotify01 as an example.
> > There are many such cases, like fanotify03.
>
> > The point is we cannot replace tst_res() with tst_brk() when only some of the
> > test cases may be supported.
>
> Sure, I'll check in all tests that tst_res() won't be replaced with tst_brk().
>
> > > But FAN_REPORT_FID in is on both files already checked after fanotify_init()
> > > call. Not sure if it must be check also for fanotify_mark(), because it's
> > > only in FANOTIFY_INIT_FLAGS (via FANOTIFY_FID_BITS). FANOTIFY_MARK_FLAGS has
> > > other flags.
>
> > > If yes, I'll probably need to create fanotify_supported_by_kernel(...), which
> > > check for all not supported flags and will be used in those 2 places and in
> > > safe_fanotify_init(). Something like this:
>
> > > typedef void (*tst_res_func_t)(const char *file, const int lineno,
> > >                 int ttype, const char *fmt, ...);
>
> > > int fanotify_flags_supported_by_kernel(const char *file, const int lineno,
> > >         unsigned int flags, int strict)
> > > {
> > >         tst_res_func_t res_func = tst_res_;
> > >         int unsupported = 0;
>
> > >         if (strict)
> > >                 res_func = tst_brk_;
>
> > >         if (errno == EINVAL) {
> > >                 if (flags & FAN_REPORT_TID) {
> > >                         tst_res_(file, lineno, TINFO,
> > >                                          "FAN_REPORT_TID not supported by kernel?");
> > >                         unsupported = 1;
> > >                 }
>
> > >                 if (flags & FAN_REPORT_FID) {
> > >                         tst_res_(file, lineno, TINFO,
> > >                                          "FAN_REPORT_FID not supported by kernel?");
> > >                         unsupported = 1;
> > >                 }
>
> > >                 if (flags & FAN_REPORT_DIR_FID) {
> > >                         tst_res_(file, lineno, TINFO,
> > >                                          "FAN_REPORT_DIR_FID not supported by kernel?");
> > >                         unsupported = 1;
> > >                 }
>
> > >                 if (flags & FAN_REPORT_NAME) {
> > >                         tst_res_(file, lineno, TINFO,
> > >                                          "FAN_REPORT_NAME not supported by kernel?");
> > >                         unsupported = 1;
> > >                 }
>
> > >                 if (unsupported)
> > >                         res_func(file, lineno, TCONF, "Unsupported configuration, see above");
> > >                 else
> > >                         tst_brk_(file, lineno, TBROK, "Unknown failure");
>
> > >                 return -1;
> > >         }
>
> > >         return 0;
> > > }
>
>
> > That seems too much and adds more noise than valuable info in many cases
> > or maybe I didn't understand.
>
> > > These are flags for fanotify_init(). Flags for fanotify_mark() are currently
> > > handled by fanotify_exec_events_supported_by_kernel() (used for FAN_OPEN_EXEC
> > > and FAN_OPEN_EXEC_PERM), using different approach. Testing fanotify_mark() flags
> > > support in advance in setup makes tests faster, I'm just not happy we use
> > > different approach. Any tip for improving this or improving readability is
> > > welcome.
>
>
> > I think the best would be to always test in advance like exec events,
> > for FAN_REPORT_ fanotify_init() flags and FAN_MARK_FILESYSTEM
> > fanotify_mark() flag whenever relevant.
>
> > I didn't go over all tests to see how that would look, but I have a feeling
> > that would end up being the cleanest approach.
>
> > Thanks,
> > Amir.
>
> OK, I'll have a look whether FAN_REPORT_* will be easy to transform to checks in
> advance.
>

Be aware.
For FAN_REPORT_FID, FAN_REPORT_FID (fanotify_init) and
FAN_MARK_FILESYSTEM (fanotify_mark) support can be tested without
any other flags, but FAN_REPORT_NAME is not valid by itself.
Instead, you can check in advance support for the flag combination
FAN_REPORT_DFID_NAME and you may also change the TCONF
warning accordingly.
There is no kernel where only one of the two flags is supported.

> I'll also try to wrote some automatic detection for testcases which use
> struct tcase (looping the struct and collect flags with & and pass it to some
> function). Maybe too complicated having to declare what is required to check
> is something which is IMHO error prone (we probably forget to update what is
> needed to be checked when we add/remove/change test structs).
>

IMHO this is overengineering.
Your cleanup is a huge improvement and big enough as is.
Please try to get it merged first without any further cleanups.
There is already a fix in upstream and stable kernel ("fanotify: fix logic of
reporting name info with watched parent") for which I have posted a test
but waiting for your cleanups to land before re-posting.

Thanks,
Amir.

[LTP] [PATCH v3 4/5] fanotify: Check FAN_REPORT_{FID, NAME} support

[Petr Vorel]

in safe_fanotify_init()

That require to move the definition after flags.

Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/syscalls/fanotify/fanotify.h | 65 +++++++++++--------
 1 file changed, 39 insertions(+), 26 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 18a2328cf..04b6699cf 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -38,32 +38,6 @@ static long fanotify_mark(int fd, unsigned int flags, uint64_t mask,
 
 #endif /* HAVE_SYS_FANOTIFY_H */
 
-int safe_fanotify_init(const char *file, const int lineno,
-	unsigned int flags, unsigned int event_f_flags)
-{
-	int rval;
-
-#ifdef HAVE_SYS_FANOTIFY_H
-	rval = fanotify_init(flags, event_f_flags);
-
-	if (rval == -1) {
-		if (errno == ENOSYS) {
-			tst_brk(TCONF,
-				"fanotify is not configured in this kernel.");
-		}
-		tst_brk(TBROK | TERRNO,
-			"%s:%d: fanotify_init() failed", file, lineno);
-	}
-#else
-	tst_brk(TCONF, "Header <sys/fanotify.h> is not present");
-#endif /* HAVE_SYS_FANOTIFY_H */
-
-	return rval;
-}
-
-#define SAFE_FANOTIFY_INIT(fan, mode)  \
-	safe_fanotify_init(__FILE__, __LINE__, (fan), (mode))
-
 #ifndef FAN_REPORT_TID
 #define FAN_REPORT_TID		0x00000100
 #endif
@@ -190,6 +164,45 @@ struct fanotify_event_info_fid {
 #define MAX_HANDLE_SZ		128
 #endif
 
+int safe_fanotify_init(const char *file, const int lineno,
+	unsigned int flags, unsigned int event_f_flags)
+{
+	int rval;
+
+#ifdef HAVE_SYS_FANOTIFY_H
+	rval = fanotify_init(flags, event_f_flags);
+
+	if (rval == -1) {
+		if (errno == ENOSYS) {
+			tst_brk(TCONF, "%s:%d: fanotify is not configured in this kernel",
+				file, lineno);
+		}
+
+		if (errno == EINVAL) {
+			if (flags & FAN_REPORT_FID) {
+				tst_brk(TCONF, "%s:%d: FAN_REPORT_FID not supported by kernel?",
+					file, lineno);
+			}
+
+			if (flags & FAN_REPORT_NAME) {
+				tst_brk(TCONF, "%s:%d: FAN_REPORT_NAME not supported by kernel?",
+					file, lineno);
+			}
+		}
+
+		tst_brk(TBROK | TERRNO, "%s:%d: fanotify_init() failed",
+			file, lineno);
+	}
+#else
+	tst_brk(TCONF, "Header <sys/fanotify.h> is not present");
+#endif /* HAVE_SYS_FANOTIFY_H */
+
+	return rval;
+}
+
+#define SAFE_FANOTIFY_INIT(fan, mode)  \
+	safe_fanotify_init(__FILE__, __LINE__, (fan), (mode))
+
 static inline int safe_fanotify_mark(const char *file, const int lineno,
 			int fd, unsigned int flags, uint64_t mask,
 			int dfd, const char *pathname)
-- 
2.29.2

[LTP] [PATCH v3 4/5] fanotify: Check FAN_REPORT_{FID, NAME} support

[Cyril Hrubis]

Hi!
> +int safe_fanotify_init(const char *file, const int lineno,
> +	unsigned int flags, unsigned int event_f_flags)
> +{
> +	int rval;
> +
> +#ifdef HAVE_SYS_FANOTIFY_H
> +	rval = fanotify_init(flags, event_f_flags);
> +
> +	if (rval == -1) {
> +		if (errno == ENOSYS) {
> +			tst_brk(TCONF, "%s:%d: fanotify is not configured in this kernel",
> +				file, lineno);
> +		}
> +
> +		if (errno == EINVAL) {
> +			if (flags & FAN_REPORT_FID) {
> +				tst_brk(TCONF, "%s:%d: FAN_REPORT_FID not supported by kernel?",
> +					file, lineno);
> +			}
> +
> +			if (flags & FAN_REPORT_NAME) {
> +				tst_brk(TCONF, "%s:%d: FAN_REPORT_NAME not supported by kernel?",
> +					file, lineno);
> +			}

If we happen to have both in flags it will report only the first one
here. So maybe we should use tst_res(TINFO, "") followed by
tst_brk(TCONF, "Unsupported configuration, see above"); or something
like that.

> +		}
> +
> +		tst_brk(TBROK | TERRNO, "%s:%d: fanotify_init() failed",
> +			file, lineno);
> +	}
> +#else
> +	tst_brk(TCONF, "Header <sys/fanotify.h> is not present");
> +#endif /* HAVE_SYS_FANOTIFY_H */
> +
> +	return rval;
> +}
> +
> +#define SAFE_FANOTIFY_INIT(fan, mode)  \
> +	safe_fanotify_init(__FILE__, __LINE__, (fan), (mode))
> +
>  static inline int safe_fanotify_mark(const char *file, const int lineno,
>  			int fd, unsigned int flags, uint64_t mask,
>  			int dfd, const char *pathname)
> -- 
> 2.29.2
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3 5/5] fanotify: Add a pedantic check for return value

[Petr Vorel]

for fanotify_init() in safe_fanotify_init()

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/syscalls/fanotify/fanotify.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 04b6699cf..dc76d092b 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -193,6 +193,11 @@ int safe_fanotify_init(const char *file, const int lineno,
 		tst_brk(TBROK | TERRNO, "%s:%d: fanotify_init() failed",
 			file, lineno);
 	}
+
+	if (rval < -1) {
+		tst_brk(TBROK | TERRNO, "%s:%d: invalid fanotify_init() return %d",
+			file, lineno, rval);
+	}
 #else
 	tst_brk(TCONF, "Header <sys/fanotify.h> is not present");
 #endif /* HAVE_SYS_FANOTIFY_H */
-- 
2.29.2

[LTP] [PATCH v3 5/5] fanotify: Add a pedantic check for return value

[Cyril Hrubis]

Hi!
This one looks obviously okay.

-- 
Cyril Hrubis
chrubis@suse.cz