All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH 0/4] Fix fanotify14
@ 2022-10-13 15:49 Martin Doucha
  2022-10-13 15:49 ` [LTP] [PATCH 1/4] fanotify14: Print human-readable test case flags Martin Doucha
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Martin Doucha @ 2022-10-13 15:49 UTC (permalink / raw)
  To: amir73il, ltp

Fanotify14 tests some fanotify_init() flags which are not supported
on older kernels but doesn't properly check for their availability.
Fix the init flag availability checks, print more verbose information
about which test case is currently being executed and move the LTP copy
of fanotify constants to an LAPI header.

Martin Doucha (4):
  fanotify14: Print human-readable test case flags
  Move fanotify fallback constants and structs to LAPI header
  Add fanotify_get_supported_init_flags() helper function
  fanotify14: Improve check for unsupported init flags

 include/lapi/fanotify.h                       | 210 +++++++++++++++++
 testcases/kernel/syscalls/fanotify/fanotify.h | 219 ++----------------
 .../kernel/syscalls/fanotify/fanotify14.c     | 208 +++++++++--------
 3 files changed, 347 insertions(+), 290 deletions(-)
 create mode 100644 include/lapi/fanotify.h

-- 
2.37.3


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

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

* [LTP] [PATCH 1/4] fanotify14: Print human-readable test case flags
  2022-10-13 15:49 [LTP] [PATCH 0/4] Fix fanotify14 Martin Doucha
@ 2022-10-13 15:49 ` Martin Doucha
  2022-10-13 21:36   ` Petr Vorel
  2022-10-13 15:49 ` [LTP] [PATCH 2/4] Move fanotify fallback constants and structs to LAPI header Martin Doucha
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Martin Doucha @ 2022-10-13 15:49 UTC (permalink / raw)
  To: amir73il, ltp

It's hard to tell which test case is failing from the current fanotify14
output. Print test case flags to make failure analysis easier.

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 .../kernel/syscalls/fanotify/fanotify14.c     | 194 ++++++++++--------
 1 file changed, 108 insertions(+), 86 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c
index 594259ccf..ee42aaf68 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify14.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify14.c
@@ -38,6 +38,8 @@
 #define INODE_EVENTS (FAN_ATTRIB | FAN_CREATE | FAN_DELETE | FAN_MOVE | \
 		      FAN_DELETE_SELF | FAN_MOVE_SELF)
 
+#define FLAGS_DESC(flags) (flags), (#flags)
+
 static int fanotify_fd;
 static int fan_report_target_fid_unsupported;
 static int ignore_mark_unsupported;
@@ -49,101 +51,113 @@ static int ignore_mark_unsupported;
  */
 static struct test_case_t {
 	unsigned int init_flags;
+	const char *init_desc;
 	unsigned int mark_flags;
+	const char *mark_desc;
 	/* zero mask expects to fail on fanotify_init() */
 	unsigned long long mask;
+	const char *mask_desc;
 	int expected_errno;
 } test_cases[] = {
-	{
-		/* FAN_REPORT_FID without class FAN_CLASS_NOTIF is not valid */
-		FAN_CLASS_CONTENT | FAN_REPORT_FID, 0, 0, EINVAL
-	},
-	{
-		/* FAN_REPORT_FID without class FAN_CLASS_NOTIF is not valid */
-		FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID, 0, 0, EINVAL
-	},
-	{
-		/* INODE_EVENTS in mask without class FAN_REPORT_FID are not valid */
-		FAN_CLASS_NOTIF, 0, INODE_EVENTS, EINVAL
-	},
-	{
-		/* INODE_EVENTS in mask with FAN_MARK_MOUNT are not valid */
-		FAN_CLASS_NOTIF | FAN_REPORT_FID, FAN_MARK_MOUNT, INODE_EVENTS, EINVAL
-	},
-	{
-		/* FAN_REPORT_NAME without FAN_REPORT_DIR_FID is not valid */
-		FAN_CLASS_NOTIF | FAN_REPORT_NAME, 0, 0, EINVAL
-	},
-	{
-		/* FAN_REPORT_NAME without FAN_REPORT_DIR_FID is not valid */
-		FAN_CLASS_NOTIF | FAN_REPORT_FID | FAN_REPORT_NAME, 0, 0, EINVAL
-	},
-	{
-		/* FAN_REPORT_TARGET_FID without FAN_REPORT_FID is not valid */
-		FAN_CLASS_NOTIF | FAN_REPORT_TARGET_FID | FAN_REPORT_DFID_NAME, 0, 0, EINVAL
-	},
-	{
-		/* FAN_REPORT_TARGET_FID without FAN_REPORT_NAME is not valid */
-		FAN_CLASS_NOTIF | FAN_REPORT_TARGET_FID | FAN_REPORT_DFID_FID, 0, 0, EINVAL
-	},
-	{
-		/* FAN_RENAME without FAN_REPORT_NAME is not valid */
-		FAN_CLASS_NOTIF | FAN_REPORT_DFID_FID, 0, FAN_RENAME, EINVAL
-	},
-	{
-		/* With FAN_MARK_ONLYDIR on non-dir is not valid */
-		FAN_CLASS_NOTIF, FAN_MARK_ONLYDIR, FAN_OPEN, ENOTDIR
-	},
-	{
-		/* With FAN_REPORT_TARGET_FID, FAN_DELETE on non-dir is not valid */
-		FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME_TARGET, 0, FAN_DELETE, ENOTDIR
-	},
-	{
-		/* With FAN_REPORT_TARGET_FID, FAN_RENAME on non-dir is not valid */
-		FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME_TARGET, 0, FAN_RENAME, ENOTDIR
-	},
-	{
-		/* With FAN_REPORT_TARGET_FID, FAN_ONDIR on non-dir is not valid */
-		FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME_TARGET, 0, FAN_OPEN | FAN_ONDIR, ENOTDIR
-	},
-	{
-		/* With FAN_REPORT_TARGET_FID, FAN_EVENT_ON_CHILD on non-dir is not valid */
-		FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME_TARGET, 0, FAN_OPEN | FAN_EVENT_ON_CHILD, ENOTDIR
-	},
-	{
-		/* FAN_MARK_IGNORE_SURV with FAN_DELETE on non-dir is not valid */
-		FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME, FAN_MARK_IGNORE_SURV, FAN_DELETE, ENOTDIR
-	},
-	{
-		/* FAN_MARK_IGNORE_SURV with FAN_RENAME on non-dir is not valid */
-		FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME, FAN_MARK_IGNORE_SURV, FAN_RENAME, ENOTDIR
-	},
-	{
-		/* FAN_MARK_IGNORE_SURV with FAN_ONDIR on non-dir is not valid */
-		FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME, FAN_MARK_IGNORE_SURV, FAN_OPEN | FAN_ONDIR, ENOTDIR
-	},
-	{
-		/* FAN_MARK_IGNORE_SURV with FAN_EVENT_ON_CHILD on non-dir is not valid */
-		FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME, FAN_MARK_IGNORE_SURV, FAN_OPEN | FAN_EVENT_ON_CHILD, ENOTDIR
-	},
-	{
-		/* FAN_MARK_IGNORE without FAN_MARK_IGNORED_SURV_MODIFY on directory is not valid */
-		FAN_CLASS_NOTIF, FAN_MARK_IGNORE, FAN_OPEN, EISDIR
-	},
-	{
-		/* FAN_MARK_IGNORE without FAN_MARK_IGNORED_SURV_MODIFY on mount mark is not valid */
-		FAN_CLASS_NOTIF, FAN_MARK_MOUNT | FAN_MARK_IGNORE, FAN_OPEN, EINVAL
-	},
-	{
-		/* FAN_MARK_IGNORE without FAN_MARK_IGNORED_SURV_MODIFY on filesystem mark is not valid */
-		FAN_CLASS_NOTIF, FAN_MARK_FILESYSTEM | FAN_MARK_IGNORE, FAN_OPEN, EINVAL
-	},
+	/* FAN_REPORT_FID without class FAN_CLASS_NOTIF is not valid */
+	{FLAGS_DESC(FAN_CLASS_CONTENT | FAN_REPORT_FID), 0, NULL, 0, NULL,
+		EINVAL},
+
+	/* FAN_REPORT_FID without class FAN_CLASS_NOTIF is not valid */
+	{FLAGS_DESC(FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID), 0, NULL, 0, NULL,
+		EINVAL},
+
+	/* INODE_EVENTS in mask without class FAN_REPORT_FID are not valid */
+	{FLAGS_DESC(FAN_CLASS_NOTIF), FLAGS_DESC(0), FLAGS_DESC(INODE_EVENTS),
+		EINVAL},
+
+	/* INODE_EVENTS in mask with FAN_MARK_MOUNT are not valid */
+	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_FID),
+		FLAGS_DESC(FAN_MARK_MOUNT), FLAGS_DESC(INODE_EVENTS), EINVAL},
+
+	/* FAN_REPORT_NAME without FAN_REPORT_DIR_FID is not valid */
+	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_NAME), 0, NULL, 0, NULL,
+		EINVAL},
+
+	/* FAN_REPORT_NAME without FAN_REPORT_DIR_FID is not valid */
+	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_FID | FAN_REPORT_NAME), 0,
+		NULL, 0, NULL, EINVAL},
+
+	/* FAN_REPORT_TARGET_FID without FAN_REPORT_FID is not valid */
+	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_TARGET_FID | FAN_REPORT_DFID_NAME),
+		0, NULL, 0, NULL, EINVAL},
+
+	/* FAN_REPORT_TARGET_FID without FAN_REPORT_NAME is not valid */
+	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_TARGET_FID | FAN_REPORT_DFID_FID),
+		0, NULL, 0, NULL, EINVAL},
+
+	/* FAN_RENAME without FAN_REPORT_NAME is not valid */
+	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_DFID_FID), FLAGS_DESC(0),
+		FLAGS_DESC(FAN_RENAME), EINVAL},
+
+	/* With FAN_MARK_ONLYDIR on non-dir is not valid */
+	{FLAGS_DESC(FAN_CLASS_NOTIF), FLAGS_DESC(FAN_MARK_ONLYDIR),
+		FLAGS_DESC(FAN_OPEN), ENOTDIR},
+
+	/* With FAN_REPORT_TARGET_FID, FAN_DELETE on non-dir is not valid */
+	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME_TARGET),
+		FLAGS_DESC(0), FLAGS_DESC(FAN_DELETE), ENOTDIR},
+
+	/* With FAN_REPORT_TARGET_FID, FAN_RENAME on non-dir is not valid */
+	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME_TARGET),
+		FLAGS_DESC(0), FLAGS_DESC(FAN_RENAME), ENOTDIR},
+
+	/* With FAN_REPORT_TARGET_FID, FAN_ONDIR on non-dir is not valid */
+	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME_TARGET),
+		FLAGS_DESC(0), FLAGS_DESC(FAN_OPEN | FAN_ONDIR), ENOTDIR},
+
+	/* With FAN_REPORT_TARGET_FID, FAN_EVENT_ON_CHILD on non-dir is not valid */
+	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME_TARGET),
+		FLAGS_DESC(0), FLAGS_DESC(FAN_OPEN | FAN_EVENT_ON_CHILD),
+		ENOTDIR},
+
+	/* FAN_MARK_IGNORE_SURV with FAN_DELETE on non-dir is not valid */
+	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME),
+		FLAGS_DESC(FAN_MARK_IGNORE_SURV), FLAGS_DESC(FAN_DELETE),
+		ENOTDIR},
+
+	/* FAN_MARK_IGNORE_SURV with FAN_RENAME on non-dir is not valid */
+	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME),
+		FLAGS_DESC(FAN_MARK_IGNORE_SURV), FLAGS_DESC(FAN_RENAME),
+		ENOTDIR},
+
+	/* FAN_MARK_IGNORE_SURV with FAN_ONDIR on non-dir is not valid */
+	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME),
+		FLAGS_DESC(FAN_MARK_IGNORE_SURV),
+		FLAGS_DESC(FAN_OPEN | FAN_ONDIR), ENOTDIR},
+
+	/* FAN_MARK_IGNORE_SURV with FAN_EVENT_ON_CHILD on non-dir is not valid */
+	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME),
+		FLAGS_DESC(FAN_MARK_IGNORE_SURV),
+		FLAGS_DESC(FAN_OPEN | FAN_EVENT_ON_CHILD), ENOTDIR},
+
+	/* FAN_MARK_IGNORE without FAN_MARK_IGNORED_SURV_MODIFY on directory is not valid */
+	{FLAGS_DESC(FAN_CLASS_NOTIF), FLAGS_DESC(FAN_MARK_IGNORE),
+		FLAGS_DESC(FAN_OPEN), EISDIR},
+
+	/* FAN_MARK_IGNORE without FAN_MARK_IGNORED_SURV_MODIFY on mount mark is not valid */
+	{FLAGS_DESC(FAN_CLASS_NOTIF),
+		FLAGS_DESC(FAN_MARK_MOUNT | FAN_MARK_IGNORE),
+		FLAGS_DESC(FAN_OPEN), EINVAL},
+
+	/* FAN_MARK_IGNORE without FAN_MARK_IGNORED_SURV_MODIFY on filesystem mark is not valid */
+	{FLAGS_DESC(FAN_CLASS_NOTIF),
+		FLAGS_DESC(FAN_MARK_FILESYSTEM | FAN_MARK_IGNORE),
+		FLAGS_DESC(FAN_OPEN), EINVAL},
 };
 
 static void do_test(unsigned int number)
 {
 	struct test_case_t *tc = &test_cases[number];
 
+	tst_res(TINFO, "Test case %d: fanotify_init(%s, O_RDONLY)", number,
+		tc->init_desc);
+
 	if (fan_report_target_fid_unsupported && tc->init_flags & FAN_REPORT_TARGET_FID) {
 		FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_TARGET_FID,
 					    fan_report_target_fid_unsupported);
@@ -155,8 +169,14 @@ static void do_test(unsigned int number)
 		return;
 	}
 
-	TST_EXP_FD_OR_FAIL(fanotify_fd = fanotify_init(tc->init_flags, O_RDONLY),
-			   !tc->mask && tc->expected_errno ? tc->expected_errno : 0);
+	if (!tc->mask && tc->expected_errno) {
+		TST_EXP_FAIL(fanotify_init(tc->init_flags, O_RDONLY),
+			tc->expected_errno);
+	} else {
+		TST_EXP_FD(fanotify_init(tc->init_flags, O_RDONLY));
+	}
+
+	fanotify_fd = TST_RET;
 
 	if (fanotify_fd < 0)
 		return;
@@ -167,6 +187,8 @@ static void do_test(unsigned int number)
 	/* Set mark on non-dir only when expecting error ENOTDIR */
 	const char *path = tc->expected_errno == ENOTDIR ? FILE1 : MNTPOINT;
 
+	tst_res(TINFO, "Testing fanotify_mark(FAN_MARK_ADD | %s, %s)",
+		tc->mark_desc, tc->mask_desc);
 	TST_EXP_FD_OR_FAIL(fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark_flags,
 					 tc->mask, AT_FDCWD, path),
 					 tc->expected_errno);
-- 
2.37.3


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

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

* [LTP] [PATCH 2/4] Move fanotify fallback constants and structs to LAPI header
  2022-10-13 15:49 [LTP] [PATCH 0/4] Fix fanotify14 Martin Doucha
  2022-10-13 15:49 ` [LTP] [PATCH 1/4] fanotify14: Print human-readable test case flags Martin Doucha
@ 2022-10-13 15:49 ` Martin Doucha
  2022-10-13 22:08   ` Petr Vorel
  2022-10-13 15:49 ` [LTP] [PATCH 3/4] Add fanotify_get_supported_init_flags() helper function Martin Doucha
  2022-10-13 15:49 ` [LTP] [PATCH 4/4] fanotify14: Improve check for unsupported init flags Martin Doucha
  3 siblings, 1 reply; 14+ messages in thread
From: Martin Doucha @ 2022-10-13 15:49 UTC (permalink / raw)
  To: amir73il, ltp

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 include/lapi/fanotify.h                       | 210 ++++++++++++++++++
 testcases/kernel/syscalls/fanotify/fanotify.h | 199 +----------------
 2 files changed, 211 insertions(+), 198 deletions(-)
 create mode 100644 include/lapi/fanotify.h

diff --git a/include/lapi/fanotify.h b/include/lapi/fanotify.h
new file mode 100644
index 000000000..e27cced3f
--- /dev/null
+++ b/include/lapi/fanotify.h
@@ -0,0 +1,210 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2012-2020 Linux Test Project.  All Rights Reserved.
+ * Author: Jan Kara, November 2013
+ */
+
+#ifndef	LAPI_FANOTIFY_H__
+#define	LAPI_FANOTIFY_H__
+
+#include "config.h"
+#include <sys/fanotify.h>
+
+#ifndef FAN_REPORT_TID
+#define FAN_REPORT_TID		0x00000100
+#endif
+#ifndef FAN_REPORT_FID
+#define FAN_REPORT_FID		0x00000200
+#endif
+#ifndef FAN_REPORT_DIR_FID
+#define FAN_REPORT_DIR_FID	0x00000400
+#endif
+#ifndef FAN_REPORT_NAME
+#define FAN_REPORT_NAME		0x00000800
+#define FAN_REPORT_DFID_NAME     (FAN_REPORT_DIR_FID | FAN_REPORT_NAME)
+#endif
+#ifndef FAN_REPORT_PIDFD
+#define FAN_REPORT_PIDFD	0x00000080
+#endif
+#ifndef FAN_REPORT_TARGET_FID
+#define FAN_REPORT_TARGET_FID	0x00001000
+#define FAN_REPORT_DFID_NAME_TARGET (FAN_REPORT_DFID_NAME | \
+				     FAN_REPORT_FID | FAN_REPORT_TARGET_FID)
+#endif
+
+/* Non-uapi convenience macros */
+#ifndef FAN_REPORT_DFID_NAME_FID
+#define FAN_REPORT_DFID_NAME_FID (FAN_REPORT_DFID_NAME | FAN_REPORT_FID)
+#endif
+#ifndef FAN_REPORT_DFID_FID
+#define FAN_REPORT_DFID_FID      (FAN_REPORT_DIR_FID | FAN_REPORT_FID)
+#endif
+
+#ifndef FAN_MARK_INODE
+#define FAN_MARK_INODE		0
+#endif
+#ifndef FAN_MARK_FILESYSTEM
+#define FAN_MARK_FILESYSTEM	0x00000100
+#endif
+#ifndef FAN_MARK_EVICTABLE
+#define FAN_MARK_EVICTABLE	0x00000200
+#endif
+#ifndef FAN_MARK_IGNORE
+#define FAN_MARK_IGNORE		0x00000400
+#endif
+#ifndef FAN_MARK_IGNORE_SURV
+#define FAN_MARK_IGNORE_SURV	(FAN_MARK_IGNORE | FAN_MARK_IGNORED_SURV_MODIFY)
+#endif
+/* Non-uapi convenience macros */
+#ifndef FAN_MARK_IGNORED_SURV
+#define FAN_MARK_IGNORED_SURV	(FAN_MARK_IGNORED_MASK | \
+				FAN_MARK_IGNORED_SURV_MODIFY)
+#endif
+#ifndef FAN_MARK_PARENT
+#define FAN_MARK_PARENT		FAN_MARK_ONLYDIR
+#endif
+#ifndef FAN_MARK_SUBDIR
+#define FAN_MARK_SUBDIR		FAN_MARK_ONLYDIR
+#endif
+#ifndef FAN_MARK_TYPES
+#define FAN_MARK_TYPES (FAN_MARK_INODE | FAN_MARK_MOUNT | FAN_MARK_FILESYSTEM)
+#endif
+
+/* New dirent event masks */
+#ifndef FAN_ATTRIB
+#define FAN_ATTRIB		0x00000004
+#endif
+#ifndef FAN_MOVED_FROM
+#define FAN_MOVED_FROM		0x00000040
+#endif
+#ifndef FAN_MOVED_TO
+#define FAN_MOVED_TO		0x00000080
+#endif
+#ifndef FAN_CREATE
+#define FAN_CREATE		0x00000100
+#endif
+#ifndef FAN_DELETE
+#define FAN_DELETE		0x00000200
+#endif
+#ifndef FAN_DELETE_SELF
+#define FAN_DELETE_SELF		0x00000400
+#endif
+#ifndef FAN_MOVE_SELF
+#define FAN_MOVE_SELF		0x00000800
+#endif
+#ifndef FAN_MOVE
+#define FAN_MOVE		(FAN_MOVED_FROM | FAN_MOVED_TO)
+#endif
+#ifndef FAN_OPEN_EXEC
+#define FAN_OPEN_EXEC		0x00001000
+#endif
+#ifndef FAN_OPEN_EXEC_PERM
+#define FAN_OPEN_EXEC_PERM	0x00040000
+#endif
+#ifndef FAN_FS_ERROR
+#define FAN_FS_ERROR		0x00008000
+#endif
+#ifndef FAN_RENAME
+#define FAN_RENAME		0x10000000
+#endif
+
+/* Additional error status codes that can be returned to userspace */
+#ifndef FAN_NOPIDFD
+#define FAN_NOPIDFD		-1
+#endif
+#ifndef FAN_EPIDFD
+#define FAN_EPIDFD		-2
+#endif
+
+/* Flags required for unprivileged user group */
+#define FANOTIFY_REQUIRED_USER_INIT_FLAGS    (FAN_REPORT_FID)
+
+/*
+ * FAN_ALL_PERM_EVENTS has been deprecated, so any new permission events
+ * are not to be added to it. To cover the instance where a new permission
+ * event is defined, we create a new macro that is to include all
+ * permission events. Any new permission events should be added to this
+ * macro.
+ */
+#define LTP_ALL_PERM_EVENTS	(FAN_OPEN_PERM | FAN_OPEN_EXEC_PERM | \
+				 FAN_ACCESS_PERM)
+
+struct fanotify_group_type {
+	unsigned int flag;
+	const char *name;
+};
+
+struct fanotify_mark_type {
+	unsigned int flag;
+	const char *name;
+};
+
+#ifndef __kernel_fsid_t
+typedef struct {
+	int	val[2];
+} lapi_fsid_t;
+#define __kernel_fsid_t lapi_fsid_t
+#endif /* __kernel_fsid_t */
+
+#ifndef FAN_EVENT_INFO_TYPE_FID
+#define FAN_EVENT_INFO_TYPE_FID		1
+#endif
+#ifndef FAN_EVENT_INFO_TYPE_DFID_NAME
+#define FAN_EVENT_INFO_TYPE_DFID_NAME	2
+#endif
+#ifndef FAN_EVENT_INFO_TYPE_DFID
+#define FAN_EVENT_INFO_TYPE_DFID	3
+#endif
+#ifndef FAN_EVENT_INFO_TYPE_PIDFD
+#define FAN_EVENT_INFO_TYPE_PIDFD	4
+#endif
+#ifndef FAN_EVENT_INFO_TYPE_ERROR
+#define FAN_EVENT_INFO_TYPE_ERROR	5
+#endif
+
+#ifndef FAN_EVENT_INFO_TYPE_OLD_DFID_NAME
+#define FAN_EVENT_INFO_TYPE_OLD_DFID_NAME	10
+#endif
+#ifndef FAN_EVENT_INFO_TYPE_NEW_DFID_NAME
+#define FAN_EVENT_INFO_TYPE_NEW_DFID_NAME	12
+#endif
+
+#ifndef HAVE_STRUCT_FANOTIFY_EVENT_INFO_HEADER
+struct fanotify_event_info_header {
+	uint8_t info_type;
+	uint8_t pad;
+	uint16_t len;
+};
+#endif /* HAVE_STRUCT_FANOTIFY_EVENT_INFO_HEADER */
+
+#ifndef HAVE_STRUCT_FANOTIFY_EVENT_INFO_FID
+struct fanotify_event_info_fid {
+	struct fanotify_event_info_header hdr;
+	__kernel_fsid_t fsid;
+	unsigned char handle[0];
+};
+#endif /* HAVE_STRUCT_FANOTIFY_EVENT_INFO_FID */
+
+#ifndef HAVE_STRUCT_FANOTIFY_EVENT_INFO_PIDFD
+struct fanotify_event_info_pidfd {
+	struct fanotify_event_info_header hdr;
+	int32_t pidfd;
+};
+#endif /* HAVE_STRUCT_FANOTIFY_EVENT_INFO_PIDFD */
+
+#ifndef HAVE_STRUCT_FANOTIFY_EVENT_INFO_ERROR
+struct fanotify_event_info_error {
+	struct fanotify_event_info_header hdr;
+	__s32 error;
+	__u32 error_count;
+};
+#endif /* HAVE_STRUCT_FANOTIFY_EVENT_INFO_ERROR */
+
+/* NOTE: only for struct fanotify_event_info_fid */
+#ifdef HAVE_STRUCT_FANOTIFY_EVENT_INFO_FID_FSID___VAL
+# define FSID_VAL_MEMBER(fsid, i) (fsid.__val[i])
+#else
+# define FSID_VAL_MEMBER(fsid, i) (fsid.val[i])
+#endif /* HAVE_STRUCT_FANOTIFY_EVENT_INFO_FID_FSID___VAL */
+
+#endif /* LAPI_FANOTIFY_H__ */
diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index ff2b4a70a..51078103e 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -7,12 +7,11 @@
 #ifndef	__FANOTIFY_H__
 #define	__FANOTIFY_H__
 
-#include "config.h"
 #include <sys/statfs.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <errno.h>
-#include <sys/fanotify.h>
+#include "lapi/fanotify.h"
 #include "lapi/fcntl.h"
 
 static inline int safe_fanotify_init(const char *file, const int lineno,
@@ -67,202 +66,6 @@ static inline int safe_fanotify_mark(const char *file, const int lineno,
 #define SAFE_FANOTIFY_INIT(fan, mode)  \
 	safe_fanotify_init(__FILE__, __LINE__, (fan), (mode))
 
-#ifndef FAN_REPORT_TID
-#define FAN_REPORT_TID		0x00000100
-#endif
-#ifndef FAN_REPORT_FID
-#define FAN_REPORT_FID		0x00000200
-#endif
-#ifndef FAN_REPORT_DIR_FID
-#define FAN_REPORT_DIR_FID	0x00000400
-#endif
-#ifndef FAN_REPORT_NAME
-#define FAN_REPORT_NAME		0x00000800
-#define FAN_REPORT_DFID_NAME     (FAN_REPORT_DIR_FID | FAN_REPORT_NAME)
-#endif
-#ifndef FAN_REPORT_PIDFD
-#define FAN_REPORT_PIDFD	0x00000080
-#endif
-#ifndef FAN_REPORT_TARGET_FID
-#define FAN_REPORT_TARGET_FID	0x00001000
-#define FAN_REPORT_DFID_NAME_TARGET (FAN_REPORT_DFID_NAME | \
-				     FAN_REPORT_FID | FAN_REPORT_TARGET_FID)
-#endif
-
-/* Non-uapi convenience macros */
-#ifndef FAN_REPORT_DFID_NAME_FID
-#define FAN_REPORT_DFID_NAME_FID (FAN_REPORT_DFID_NAME | FAN_REPORT_FID)
-#endif
-#ifndef FAN_REPORT_DFID_FID
-#define FAN_REPORT_DFID_FID      (FAN_REPORT_DIR_FID | FAN_REPORT_FID)
-#endif
-
-#ifndef FAN_MARK_INODE
-#define FAN_MARK_INODE		0
-#endif
-#ifndef FAN_MARK_FILESYSTEM
-#define FAN_MARK_FILESYSTEM	0x00000100
-#endif
-#ifndef FAN_MARK_EVICTABLE
-#define FAN_MARK_EVICTABLE	0x00000200
-#endif
-#ifndef FAN_MARK_IGNORE
-#define FAN_MARK_IGNORE		0x00000400
-#endif
-#ifndef FAN_MARK_IGNORE_SURV
-#define FAN_MARK_IGNORE_SURV	(FAN_MARK_IGNORE | FAN_MARK_IGNORED_SURV_MODIFY)
-#endif
-/* Non-uapi convenience macros */
-#ifndef FAN_MARK_IGNORED_SURV
-#define FAN_MARK_IGNORED_SURV	(FAN_MARK_IGNORED_MASK | FAN_MARK_IGNORED_SURV_MODIFY)
-#endif
-#ifndef FAN_MARK_PARENT
-#define FAN_MARK_PARENT		FAN_MARK_ONLYDIR
-#endif
-#ifndef FAN_MARK_SUBDIR
-#define FAN_MARK_SUBDIR		FAN_MARK_ONLYDIR
-#endif
-#ifndef FAN_MARK_TYPES
-#define FAN_MARK_TYPES (FAN_MARK_INODE | FAN_MARK_MOUNT | FAN_MARK_FILESYSTEM)
-#endif
-
-/* New dirent event masks */
-#ifndef FAN_ATTRIB
-#define FAN_ATTRIB		0x00000004
-#endif
-#ifndef FAN_MOVED_FROM
-#define FAN_MOVED_FROM		0x00000040
-#endif
-#ifndef FAN_MOVED_TO
-#define FAN_MOVED_TO		0x00000080
-#endif
-#ifndef FAN_CREATE
-#define FAN_CREATE		0x00000100
-#endif
-#ifndef FAN_DELETE
-#define FAN_DELETE		0x00000200
-#endif
-#ifndef FAN_DELETE_SELF
-#define FAN_DELETE_SELF		0x00000400
-#endif
-#ifndef FAN_MOVE_SELF
-#define FAN_MOVE_SELF		0x00000800
-#endif
-#ifndef FAN_MOVE
-#define FAN_MOVE		(FAN_MOVED_FROM | FAN_MOVED_TO)
-#endif
-#ifndef FAN_OPEN_EXEC
-#define FAN_OPEN_EXEC		0x00001000
-#endif
-#ifndef FAN_OPEN_EXEC_PERM
-#define FAN_OPEN_EXEC_PERM	0x00040000
-#endif
-#ifndef FAN_FS_ERROR
-#define FAN_FS_ERROR		0x00008000
-#endif
-#ifndef FAN_RENAME
-#define FAN_RENAME		0x10000000
-#endif
-
-/* Additional error status codes that can be returned to userspace */
-#ifndef FAN_NOPIDFD
-#define FAN_NOPIDFD		-1
-#endif
-#ifndef FAN_EPIDFD
-#define FAN_EPIDFD		-2
-#endif
-
-/* Flags required for unprivileged user group */
-#define FANOTIFY_REQUIRED_USER_INIT_FLAGS    (FAN_REPORT_FID)
-
-/*
- * FAN_ALL_PERM_EVENTS has been deprecated, so any new permission events
- * are not to be added to it. To cover the instance where a new permission
- * event is defined, we create a new macro that is to include all
- * permission events. Any new permission events should be added to this
- * macro.
- */
-#define LTP_ALL_PERM_EVENTS	(FAN_OPEN_PERM | FAN_OPEN_EXEC_PERM | \
-				 FAN_ACCESS_PERM)
-
-struct fanotify_group_type {
-	unsigned int flag;
-	const char *name;
-};
-
-struct fanotify_mark_type {
-	unsigned int flag;
-	const char *name;
-};
-
-#ifndef __kernel_fsid_t
-typedef struct {
-	int	val[2];
-} lapi_fsid_t;
-#define __kernel_fsid_t lapi_fsid_t
-#endif /* __kernel_fsid_t */
-
-#ifndef FAN_EVENT_INFO_TYPE_FID
-#define FAN_EVENT_INFO_TYPE_FID		1
-#endif
-#ifndef FAN_EVENT_INFO_TYPE_DFID_NAME
-#define FAN_EVENT_INFO_TYPE_DFID_NAME	2
-#endif
-#ifndef FAN_EVENT_INFO_TYPE_DFID
-#define FAN_EVENT_INFO_TYPE_DFID	3
-#endif
-#ifndef FAN_EVENT_INFO_TYPE_PIDFD
-#define FAN_EVENT_INFO_TYPE_PIDFD	4
-#endif
-#ifndef FAN_EVENT_INFO_TYPE_ERROR
-#define FAN_EVENT_INFO_TYPE_ERROR	5
-#endif
-
-#ifndef FAN_EVENT_INFO_TYPE_OLD_DFID_NAME
-#define FAN_EVENT_INFO_TYPE_OLD_DFID_NAME	10
-#endif
-#ifndef FAN_EVENT_INFO_TYPE_NEW_DFID_NAME
-#define FAN_EVENT_INFO_TYPE_NEW_DFID_NAME	12
-#endif
-
-#ifndef HAVE_STRUCT_FANOTIFY_EVENT_INFO_HEADER
-struct fanotify_event_info_header {
-	uint8_t info_type;
-	uint8_t pad;
-	uint16_t len;
-};
-#endif /* HAVE_STRUCT_FANOTIFY_EVENT_INFO_HEADER */
-
-#ifndef HAVE_STRUCT_FANOTIFY_EVENT_INFO_FID
-struct fanotify_event_info_fid {
-	struct fanotify_event_info_header hdr;
-	__kernel_fsid_t fsid;
-	unsigned char handle[0];
-};
-#endif /* HAVE_STRUCT_FANOTIFY_EVENT_INFO_FID */
-
-#ifndef HAVE_STRUCT_FANOTIFY_EVENT_INFO_PIDFD
-struct fanotify_event_info_pidfd {
-	struct fanotify_event_info_header hdr;
-	int32_t pidfd;
-};
-#endif /* HAVE_STRUCT_FANOTIFY_EVENT_INFO_PIDFD */
-
-#ifndef HAVE_STRUCT_FANOTIFY_EVENT_INFO_ERROR
-struct fanotify_event_info_error {
-	struct fanotify_event_info_header hdr;
-	__s32 error;
-	__u32 error_count;
-};
-#endif /* HAVE_STRUCT_FANOTIFY_EVENT_INFO_ERROR */
-
-/* NOTE: only for struct fanotify_event_info_fid */
-#ifdef HAVE_STRUCT_FANOTIFY_EVENT_INFO_FID_FSID___VAL
-# define FSID_VAL_MEMBER(fsid, i) (fsid.__val[i])
-#else
-# define FSID_VAL_MEMBER(fsid, i) (fsid.val[i])
-#endif /* HAVE_STRUCT_FANOTIFY_EVENT_INFO_FID_FSID___VAL */
-
 #ifdef HAVE_NAME_TO_HANDLE_AT
 
 #ifndef MAX_HANDLE_SZ
-- 
2.37.3


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

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

* [LTP] [PATCH 3/4] Add fanotify_get_supported_init_flags() helper function
  2022-10-13 15:49 [LTP] [PATCH 0/4] Fix fanotify14 Martin Doucha
  2022-10-13 15:49 ` [LTP] [PATCH 1/4] fanotify14: Print human-readable test case flags Martin Doucha
  2022-10-13 15:49 ` [LTP] [PATCH 2/4] Move fanotify fallback constants and structs to LAPI header Martin Doucha
@ 2022-10-13 15:49 ` Martin Doucha
  2022-10-14  6:15   ` Petr Vorel
  2022-10-14  6:50   ` Amir Goldstein
  2022-10-13 15:49 ` [LTP] [PATCH 4/4] fanotify14: Improve check for unsupported init flags Martin Doucha
  3 siblings, 2 replies; 14+ messages in thread
From: Martin Doucha @ 2022-10-13 15:49 UTC (permalink / raw)
  To: amir73il, ltp

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 testcases/kernel/syscalls/fanotify/fanotify.h | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 51078103e..43434f6ac 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -213,6 +213,26 @@ static inline int fanotify_init_flags_supported_by_kernel(unsigned int flags)
 	return fanotify_init_flags_supported_on_fs(flags, NULL);
 }
 
+/*
+ * Check support of given init flags one by one and return those which are
+ * supported.
+ */
+static inline unsigned int fanotify_get_supported_init_flags(unsigned int flags,
+	const char *fname)
+{
+	unsigned int flg, ret = 0;
+
+	for (flg = 1; flg; flg <<= 1) {
+		if (!(flags & flg))
+			continue;
+
+		if (!fanotify_init_flags_supported_on_fs(flg, fname))
+			ret |= flg;
+	}
+
+	return ret;
+}
+
 typedef void (*tst_res_func_t)(const char *file, const int lineno,
 			       int ttype, const char *fmt, ...);
 
-- 
2.37.3


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

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

* [LTP] [PATCH 4/4] fanotify14: Improve check for unsupported init flags
  2022-10-13 15:49 [LTP] [PATCH 0/4] Fix fanotify14 Martin Doucha
                   ` (2 preceding siblings ...)
  2022-10-13 15:49 ` [LTP] [PATCH 3/4] Add fanotify_get_supported_init_flags() helper function Martin Doucha
@ 2022-10-13 15:49 ` Martin Doucha
  2022-10-14  6:20   ` Petr Vorel
  3 siblings, 1 reply; 14+ messages in thread
From: Martin Doucha @ 2022-10-13 15:49 UTC (permalink / raw)
  To: amir73il, ltp

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 testcases/kernel/syscalls/fanotify/fanotify14.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c
index ee42aaf68..b98ec4652 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify14.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify14.c
@@ -41,8 +41,8 @@
 #define FLAGS_DESC(flags) (flags), (#flags)
 
 static int fanotify_fd;
-static int fan_report_target_fid_unsupported;
 static int ignore_mark_unsupported;
+static unsigned int supported_init_flags;
 
 /*
  * Each test case has been designed in a manner whereby the values defined
@@ -158,9 +158,8 @@ static void do_test(unsigned int number)
 	tst_res(TINFO, "Test case %d: fanotify_init(%s, O_RDONLY)", number,
 		tc->init_desc);
 
-	if (fan_report_target_fid_unsupported && tc->init_flags & FAN_REPORT_TARGET_FID) {
-		FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_TARGET_FID,
-					    fan_report_target_fid_unsupported);
+	if (tc->init_flags & ~supported_init_flags) {
+		tst_res(TCONF, "Unsupported init flags");
 		return;
 	}
 
@@ -223,11 +222,14 @@ out:
 
 static void do_setup(void)
 {
+	unsigned int all_init_flags = FAN_REPORT_DFID_NAME_TARGET;
+
 	/* Require FAN_REPORT_FID support for all tests to simplify per test case requirements */
 	REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_FID, MNTPOINT);
 
-	fan_report_target_fid_unsupported =
-		fanotify_init_flags_supported_on_fs(FAN_REPORT_DFID_NAME_TARGET, MNTPOINT);
+	supported_init_flags = fanotify_get_supported_init_flags(all_init_flags,
+		MNTPOINT);
+
 	ignore_mark_unsupported = fanotify_mark_supported_by_kernel(FAN_MARK_IGNORE_SURV);
 
 	/* Create temporary test file to place marks on */
-- 
2.37.3


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

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

* Re: [LTP] [PATCH 1/4] fanotify14: Print human-readable test case flags
  2022-10-13 15:49 ` [LTP] [PATCH 1/4] fanotify14: Print human-readable test case flags Martin Doucha
@ 2022-10-13 21:36   ` Petr Vorel
  2022-10-14 15:49     ` Martin Doucha
  0 siblings, 1 reply; 14+ messages in thread
From: Petr Vorel @ 2022-10-13 21:36 UTC (permalink / raw)
  To: Martin Doucha; +Cc: ltp

Hi Martin,

> It's hard to tell which test case is failing from the current fanotify14
> output. Print test case flags to make failure analysis easier.

> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
> ---
>  .../kernel/syscalls/fanotify/fanotify14.c     | 194 ++++++++++--------
>  1 file changed, 108 insertions(+), 86 deletions(-)

> diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c
> index 594259ccf..ee42aaf68 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify14.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify14.c
> @@ -38,6 +38,8 @@
>  #define INODE_EVENTS (FAN_ATTRIB | FAN_CREATE | FAN_DELETE | FAN_MOVE | \
>  		      FAN_DELETE_SELF | FAN_MOVE_SELF)

> +#define FLAGS_DESC(flags) (flags), (#flags)
+1 for add ing description. But macro like this gets false positive in make
check:

fanotify14.c:41: ERROR: Macros with complex values should be enclosed in parentheses

Also quite recently, in dbb9db6ec ("syscalls/fanotify09: Make test case
definitions more readable") was single test migrated to use
.foo = value, .bar = value struct setup. This is about source code readability,
you aim for test output readability, IMHO both is important.


>  static int fanotify_fd;
>  static int fan_report_target_fid_unsupported;
>  static int ignore_mark_unsupported;
> @@ -49,101 +51,113 @@ static int ignore_mark_unsupported;
>   */
>  static struct test_case_t {
>  	unsigned int init_flags;
> +	const char *init_desc;
>  	unsigned int mark_flags;
> +	const char *mark_desc;
>  	/* zero mask expects to fail on fanotify_init() */
>  	unsigned long long mask;
> +	const char *mask_desc;
>  	int expected_errno;
>  } test_cases[] = {
> -	{
> -		/* FAN_REPORT_FID without class FAN_CLASS_NOTIF is not valid */
> -		FAN_CLASS_CONTENT | FAN_REPORT_FID, 0, 0, EINVAL
> -	},
> -	{
> -		/* FAN_REPORT_FID without class FAN_CLASS_NOTIF is not valid */
> -		FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID, 0, 0, EINVAL
> -	},
> -	{
> -		/* INODE_EVENTS in mask without class FAN_REPORT_FID are not valid */
> -		FAN_CLASS_NOTIF, 0, INODE_EVENTS, EINVAL
> -	},
> -	{
> -		/* INODE_EVENTS in mask with FAN_MARK_MOUNT are not valid */
> -		FAN_CLASS_NOTIF | FAN_REPORT_FID, FAN_MARK_MOUNT, INODE_EVENTS, EINVAL
> -	},
> -	{
> -		/* FAN_REPORT_NAME without FAN_REPORT_DIR_FID is not valid */
> -		FAN_CLASS_NOTIF | FAN_REPORT_NAME, 0, 0, EINVAL
> -	},
> -	{
> -		/* FAN_REPORT_NAME without FAN_REPORT_DIR_FID is not valid */
> -		FAN_CLASS_NOTIF | FAN_REPORT_FID | FAN_REPORT_NAME, 0, 0, EINVAL
> -	},
> -	{
> -		/* FAN_REPORT_TARGET_FID without FAN_REPORT_FID is not valid */
> -		FAN_CLASS_NOTIF | FAN_REPORT_TARGET_FID | FAN_REPORT_DFID_NAME, 0, 0, EINVAL
> -	},
> -	{
> -		/* FAN_REPORT_TARGET_FID without FAN_REPORT_NAME is not valid */
> -		FAN_CLASS_NOTIF | FAN_REPORT_TARGET_FID | FAN_REPORT_DFID_FID, 0, 0, EINVAL
> -	},
> -	{
> -		/* FAN_RENAME without FAN_REPORT_NAME is not valid */
> -		FAN_CLASS_NOTIF | FAN_REPORT_DFID_FID, 0, FAN_RENAME, EINVAL
> -	},
> -	{
> -		/* With FAN_MARK_ONLYDIR on non-dir is not valid */
> -		FAN_CLASS_NOTIF, FAN_MARK_ONLYDIR, FAN_OPEN, ENOTDIR
> -	},
> -	{
> -		/* With FAN_REPORT_TARGET_FID, FAN_DELETE on non-dir is not valid */
> -		FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME_TARGET, 0, FAN_DELETE, ENOTDIR
> -	},
> -	{
> -		/* With FAN_REPORT_TARGET_FID, FAN_RENAME on non-dir is not valid */
> -		FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME_TARGET, 0, FAN_RENAME, ENOTDIR
> -	},
> -	{
> -		/* With FAN_REPORT_TARGET_FID, FAN_ONDIR on non-dir is not valid */
> -		FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME_TARGET, 0, FAN_OPEN | FAN_ONDIR, ENOTDIR
> -	},
> -	{
> -		/* With FAN_REPORT_TARGET_FID, FAN_EVENT_ON_CHILD on non-dir is not valid */
> -		FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME_TARGET, 0, FAN_OPEN | FAN_EVENT_ON_CHILD, ENOTDIR
> -	},
> -	{
> -		/* FAN_MARK_IGNORE_SURV with FAN_DELETE on non-dir is not valid */
> -		FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME, FAN_MARK_IGNORE_SURV, FAN_DELETE, ENOTDIR
> -	},
> -	{
> -		/* FAN_MARK_IGNORE_SURV with FAN_RENAME on non-dir is not valid */
> -		FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME, FAN_MARK_IGNORE_SURV, FAN_RENAME, ENOTDIR
> -	},
> -	{
> -		/* FAN_MARK_IGNORE_SURV with FAN_ONDIR on non-dir is not valid */
> -		FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME, FAN_MARK_IGNORE_SURV, FAN_OPEN | FAN_ONDIR, ENOTDIR
> -	},
> -	{
> -		/* FAN_MARK_IGNORE_SURV with FAN_EVENT_ON_CHILD on non-dir is not valid */
> -		FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME, FAN_MARK_IGNORE_SURV, FAN_OPEN | FAN_EVENT_ON_CHILD, ENOTDIR
> -	},
> -	{
> -		/* FAN_MARK_IGNORE without FAN_MARK_IGNORED_SURV_MODIFY on directory is not valid */
> -		FAN_CLASS_NOTIF, FAN_MARK_IGNORE, FAN_OPEN, EISDIR
> -	},
> -	{
> -		/* FAN_MARK_IGNORE without FAN_MARK_IGNORED_SURV_MODIFY on mount mark is not valid */
> -		FAN_CLASS_NOTIF, FAN_MARK_MOUNT | FAN_MARK_IGNORE, FAN_OPEN, EINVAL
> -	},
> -	{
> -		/* FAN_MARK_IGNORE without FAN_MARK_IGNORED_SURV_MODIFY on filesystem mark is not valid */
> -		FAN_CLASS_NOTIF, FAN_MARK_FILESYSTEM | FAN_MARK_IGNORE, FAN_OPEN, EINVAL
> -	},
> +	/* FAN_REPORT_FID without class FAN_CLASS_NOTIF is not valid */
> +	{FLAGS_DESC(FAN_CLASS_CONTENT | FAN_REPORT_FID), 0, NULL, 0, NULL,
> +		EINVAL},
> +
> +	/* FAN_REPORT_FID without class FAN_CLASS_NOTIF is not valid */
> +	{FLAGS_DESC(FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID), 0, NULL, 0, NULL,
> +		EINVAL},
> +
> +	/* INODE_EVENTS in mask without class FAN_REPORT_FID are not valid */
> +	{FLAGS_DESC(FAN_CLASS_NOTIF), FLAGS_DESC(0), FLAGS_DESC(INODE_EVENTS),
> +		EINVAL},
> +
> +	/* INODE_EVENTS in mask with FAN_MARK_MOUNT are not valid */
> +	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_FID),
> +		FLAGS_DESC(FAN_MARK_MOUNT), FLAGS_DESC(INODE_EVENTS), EINVAL},
> +
> +	/* FAN_REPORT_NAME without FAN_REPORT_DIR_FID is not valid */
> +	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_NAME), 0, NULL, 0, NULL,
> +		EINVAL},
> +
> +	/* FAN_REPORT_NAME without FAN_REPORT_DIR_FID is not valid */
> +	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_FID | FAN_REPORT_NAME), 0,
> +		NULL, 0, NULL, EINVAL},
> +
> +	/* FAN_REPORT_TARGET_FID without FAN_REPORT_FID is not valid */
> +	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_TARGET_FID | FAN_REPORT_DFID_NAME),
> +		0, NULL, 0, NULL, EINVAL},
> +
> +	/* FAN_REPORT_TARGET_FID without FAN_REPORT_NAME is not valid */
> +	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_TARGET_FID | FAN_REPORT_DFID_FID),
> +		0, NULL, 0, NULL, EINVAL},
> +
> +	/* FAN_RENAME without FAN_REPORT_NAME is not valid */
> +	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_DFID_FID), FLAGS_DESC(0),
> +		FLAGS_DESC(FAN_RENAME), EINVAL},
> +
> +	/* With FAN_MARK_ONLYDIR on non-dir is not valid */
> +	{FLAGS_DESC(FAN_CLASS_NOTIF), FLAGS_DESC(FAN_MARK_ONLYDIR),
> +		FLAGS_DESC(FAN_OPEN), ENOTDIR},
> +
> +	/* With FAN_REPORT_TARGET_FID, FAN_DELETE on non-dir is not valid */
> +	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME_TARGET),
> +		FLAGS_DESC(0), FLAGS_DESC(FAN_DELETE), ENOTDIR},
> +
> +	/* With FAN_REPORT_TARGET_FID, FAN_RENAME on non-dir is not valid */
> +	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME_TARGET),
> +		FLAGS_DESC(0), FLAGS_DESC(FAN_RENAME), ENOTDIR},
> +
> +	/* With FAN_REPORT_TARGET_FID, FAN_ONDIR on non-dir is not valid */
> +	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME_TARGET),
> +		FLAGS_DESC(0), FLAGS_DESC(FAN_OPEN | FAN_ONDIR), ENOTDIR},
> +
> +	/* With FAN_REPORT_TARGET_FID, FAN_EVENT_ON_CHILD on non-dir is not valid */
> +	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME_TARGET),
> +		FLAGS_DESC(0), FLAGS_DESC(FAN_OPEN | FAN_EVENT_ON_CHILD),
> +		ENOTDIR},
> +
> +	/* FAN_MARK_IGNORE_SURV with FAN_DELETE on non-dir is not valid */
> +	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME),
> +		FLAGS_DESC(FAN_MARK_IGNORE_SURV), FLAGS_DESC(FAN_DELETE),
> +		ENOTDIR},
> +
> +	/* FAN_MARK_IGNORE_SURV with FAN_RENAME on non-dir is not valid */
> +	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME),
> +		FLAGS_DESC(FAN_MARK_IGNORE_SURV), FLAGS_DESC(FAN_RENAME),
> +		ENOTDIR},
> +
> +	/* FAN_MARK_IGNORE_SURV with FAN_ONDIR on non-dir is not valid */
> +	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME),
> +		FLAGS_DESC(FAN_MARK_IGNORE_SURV),
> +		FLAGS_DESC(FAN_OPEN | FAN_ONDIR), ENOTDIR},
> +
> +	/* FAN_MARK_IGNORE_SURV with FAN_EVENT_ON_CHILD on non-dir is not valid */
> +	{FLAGS_DESC(FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME),
> +		FLAGS_DESC(FAN_MARK_IGNORE_SURV),
> +		FLAGS_DESC(FAN_OPEN | FAN_EVENT_ON_CHILD), ENOTDIR},
> +
> +	/* FAN_MARK_IGNORE without FAN_MARK_IGNORED_SURV_MODIFY on directory is not valid */
> +	{FLAGS_DESC(FAN_CLASS_NOTIF), FLAGS_DESC(FAN_MARK_IGNORE),
> +		FLAGS_DESC(FAN_OPEN), EISDIR},
> +
> +	/* FAN_MARK_IGNORE without FAN_MARK_IGNORED_SURV_MODIFY on mount mark is not valid */
> +	{FLAGS_DESC(FAN_CLASS_NOTIF),
> +		FLAGS_DESC(FAN_MARK_MOUNT | FAN_MARK_IGNORE),
> +		FLAGS_DESC(FAN_OPEN), EINVAL},
> +
> +	/* FAN_MARK_IGNORE without FAN_MARK_IGNORED_SURV_MODIFY on filesystem mark is not valid */
> +	{FLAGS_DESC(FAN_CLASS_NOTIF),
> +		FLAGS_DESC(FAN_MARK_FILESYSTEM | FAN_MARK_IGNORE),
> +		FLAGS_DESC(FAN_OPEN), EINVAL},
>  };

>  static void do_test(unsigned int number)
>  {
>  	struct test_case_t *tc = &test_cases[number];

> +	tst_res(TINFO, "Test case %d: fanotify_init(%s, O_RDONLY)", number,
> +		tc->init_desc);
> +
>  	if (fan_report_target_fid_unsupported && tc->init_flags & FAN_REPORT_TARGET_FID) {
>  		FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_TARGET_FID,
>  					    fan_report_target_fid_unsupported);
> @@ -155,8 +169,14 @@ static void do_test(unsigned int number)
>  		return;
>  	}

> -	TST_EXP_FD_OR_FAIL(fanotify_fd = fanotify_init(tc->init_flags, O_RDONLY),
> -			   !tc->mask && tc->expected_errno ? tc->expected_errno : 0);
TST_EXP_FD_OR_FAIL was added only to be used by fanotify tests.
What's wrong with it?

Kind regards,
Petr

> +	if (!tc->mask && tc->expected_errno) {
> +		TST_EXP_FAIL(fanotify_init(tc->init_flags, O_RDONLY),
> +			tc->expected_errno);
> +	} else {
> +		TST_EXP_FD(fanotify_init(tc->init_flags, O_RDONLY));
> +	}
> +
> +	fanotify_fd = TST_RET;

>  	if (fanotify_fd < 0)
>  		return;
> @@ -167,6 +187,8 @@ static void do_test(unsigned int number)
>  	/* Set mark on non-dir only when expecting error ENOTDIR */
>  	const char *path = tc->expected_errno == ENOTDIR ? FILE1 : MNTPOINT;

> +	tst_res(TINFO, "Testing fanotify_mark(FAN_MARK_ADD | %s, %s)",
> +		tc->mark_desc, tc->mask_desc);
>  	TST_EXP_FD_OR_FAIL(fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark_flags,
>  					 tc->mask, AT_FDCWD, path),
>  					 tc->expected_errno);

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

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

* Re: [LTP] [PATCH 2/4] Move fanotify fallback constants and structs to LAPI header
  2022-10-13 15:49 ` [LTP] [PATCH 2/4] Move fanotify fallback constants and structs to LAPI header Martin Doucha
@ 2022-10-13 22:08   ` Petr Vorel
  2022-10-14 15:56     ` Martin Doucha
  0 siblings, 1 reply; 14+ messages in thread
From: Petr Vorel @ 2022-10-13 22:08 UTC (permalink / raw)
  To: Martin Doucha; +Cc: ltp

Hi Martin,

although it will be used only by tests in testcases/kernel/syscalls/fanotify/,
it's cleaner to follow LTP approach.

Reviewed-by: Petr Vorel <petr.vorel@gmail.com>

...
> --- /dev/null
> +++ b/include/lapi/fanotify.h
> @@ -0,0 +1,210 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (c) 2012-2020 Linux Test Project.  All Rights Reserved.
FAN_FS_ERROR has been added in 3db153665 ("syscalls/fanotify22: Introduce FAN_FS_ERROR test")
which is 2022. Can be changed before merge.

Kind regards,
Petr

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

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

* Re: [LTP] [PATCH 3/4] Add fanotify_get_supported_init_flags() helper function
  2022-10-13 15:49 ` [LTP] [PATCH 3/4] Add fanotify_get_supported_init_flags() helper function Martin Doucha
@ 2022-10-14  6:15   ` Petr Vorel
  2022-10-14  6:50   ` Amir Goldstein
  1 sibling, 0 replies; 14+ messages in thread
From: Petr Vorel @ 2022-10-14  6:15 UTC (permalink / raw)
  To: Martin Doucha; +Cc: ltp

Hi Martin,

interesting unique solution. Amir, WDYT?

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

Kind regards,
Petr

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

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

* Re: [LTP] [PATCH 4/4] fanotify14: Improve check for unsupported init flags
  2022-10-13 15:49 ` [LTP] [PATCH 4/4] fanotify14: Improve check for unsupported init flags Martin Doucha
@ 2022-10-14  6:20   ` Petr Vorel
  0 siblings, 0 replies; 14+ messages in thread
From: Petr Vorel @ 2022-10-14  6:20 UTC (permalink / raw)
  To: Martin Doucha; +Cc: ltp

Hi Martin,

right, you're fixing a regression introduced by me.
Thank you!

Fixes: fe31bfcac ("fanotify14: Use TST_EXP_FD_OR_FAIL()")

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

Kind regards,
Petr

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

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

* Re: [LTP] [PATCH 3/4] Add fanotify_get_supported_init_flags() helper function
  2022-10-13 15:49 ` [LTP] [PATCH 3/4] Add fanotify_get_supported_init_flags() helper function Martin Doucha
  2022-10-14  6:15   ` Petr Vorel
@ 2022-10-14  6:50   ` Amir Goldstein
  1 sibling, 0 replies; 14+ messages in thread
From: Amir Goldstein @ 2022-10-14  6:50 UTC (permalink / raw)
  To: Martin Doucha; +Cc: Jan Kara, ltp

On Thu, Oct 13, 2022 at 6:49 PM Martin Doucha <mdoucha@suse.cz> wrote:
>

Hi Martin,

Please refrain from empty commit messages.
The text in your cover letter would have been useful in this commit message
and in the commit message of patch 4/4.
If it were me, I would squash those two patches
to emphasise the usability aspect of this helper, but up to you.

> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
> ---
>  testcases/kernel/syscalls/fanotify/fanotify.h | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> index 51078103e..43434f6ac 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> @@ -213,6 +213,26 @@ static inline int fanotify_init_flags_supported_by_kernel(unsigned int flags)
>         return fanotify_init_flags_supported_on_fs(flags, NULL);
>  }
>
> +/*
> + * Check support of given init flags one by one and return those which are
> + * supported.
> + */
> +static inline unsigned int fanotify_get_supported_init_flags(unsigned int flags,
> +       const char *fname)
> +{
> +       unsigned int flg, ret = 0;
> +
> +       for (flg = 1; flg; flg <<= 1) {
> +               if (!(flags & flg))
> +                       continue;
> +
> +               if (!fanotify_init_flags_supported_on_fs(flg, fname))
> +                       ret |= flg;
> +       }
> +
> +       return ret;
> +}
> +

I am afraid that you have tried to generalize too much.
As a generic helper, this is wrong code, because in many cases
init flags depend on each other, for example,
FAN_REOPRT_NAME is not supported without FAN_REPORT_FID
so the test for support on every single bit independently is plain wrong.

If the code would have checked:

    fanotify_init_flags_supported_on_fs(ret | flg, fname))

It would have least been correct for the private case on fanotify
which tests the support for the flag combination
FAN_REPORT_DFID_NAME_TARGET

But the general helper would still be incorrect.

The reason that the helper would work for fanotify14
is because the chronological order in which kernel support was
added to flags (_FID, _DIR_FID, _NAME, _TARGE_FID)
matches the order of the flag bits.

So my recommendation is not to attempt to guess the
supported flag combinations, but test the relevant flag
combinations explicitly.

1. Change the return value of fanotify_init_flags_supported_on_fs()
    to <the set of supported flags> or 0
2. Change all code if (fan_report_*_unsupported) to if (!fan_report_*_supported)
    or better yet if (tc->init_flags & ~fan_report_*_supported) as in patch 4/4
3. fanotify14 setup() code would then look like this:

-        /* Require FAN_REPORT_FID support for all tests to simplify
per test case requirements */
-        REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_FID, MNTPOINT);
-
-       fan_report_target_fid_unsupported =
-
fanotify_init_flags_supported_on_fs(FAN_REPORT_DFID_NAME_TARGET,
MNTPOINT);
+       supported_init_flags =
fanotify_init_flags_supported_on_fs(FAN_REPORT_FID,
+               MNTPOINT);
+       supported_init_flags |=
fanotify_init_flags_supported_on_fs(FAN_REPORT_DFID_NAME,
+               MNTPOINT);
+       supported_init_flags |=
fanotify_init_flags_supported_on_fs(FAN_REPORT_TARGET_FID,
+               MNTPOINT);

Checking these combinations is sufficient and is more readable IMO.
This approach makes extending tests easy.

In the development history of fanotify14, commit 9df7f38c8
  syscalls/fanotify14: Test cases for FAN_REPORT_DFID_NAME
would have added the setup check for supported FAN_REPORT_DFID_NAME

and commit 66d406407
  syscalls/fanotify14: Add tests for FAN_REPORT_TARGET_FID and FAN_RENAME
would have added the setup check for supported FAN_REPORT_TARGET_FID

But the check for supported flags per test case would not need to change
if it was generic like in patch 4/4.

Thanks,
Amir.

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

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

* Re: [LTP] [PATCH 1/4] fanotify14: Print human-readable test case flags
  2022-10-13 21:36   ` Petr Vorel
@ 2022-10-14 15:49     ` Martin Doucha
  2022-10-14 16:17       ` Petr Vorel
  0 siblings, 1 reply; 14+ messages in thread
From: Martin Doucha @ 2022-10-14 15:49 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

On 13. 10. 22 23:36, Petr Vorel wrote:
> Hi Martin,
> 
>> It's hard to tell which test case is failing from the current fanotify14
>> output. Print test case flags to make failure analysis easier.
> 
>> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
>> ---
>>   .../kernel/syscalls/fanotify/fanotify14.c     | 194 ++++++++++--------
>>   1 file changed, 108 insertions(+), 86 deletions(-)
> 
>> diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c
>> index 594259ccf..ee42aaf68 100644
>> --- a/testcases/kernel/syscalls/fanotify/fanotify14.c
>> +++ b/testcases/kernel/syscalls/fanotify/fanotify14.c
>> @@ -38,6 +38,8 @@
>>   #define INODE_EVENTS (FAN_ATTRIB | FAN_CREATE | FAN_DELETE | FAN_MOVE | \
>>   		      FAN_DELETE_SELF | FAN_MOVE_SELF)
> 
>> +#define FLAGS_DESC(flags) (flags), (#flags)
> +1 for add ing description. But macro like this gets false positive in make
> check:
> 
> fanotify14.c:41: ERROR: Macros with complex values should be enclosed in parentheses
> 
> Also quite recently, in dbb9db6ec ("syscalls/fanotify09: Make test case
> definitions more readable") was single test migrated to use
> .foo = value, .bar = value struct setup. This is about source code readability,
> you aim for test output readability, IMHO both is important.

I can't use that approach here because I'm using the macro to initialize 
3 different pairs of attributes in the same structure. What I could do 
is change the flags/desc pairs into a nested struct of
{unsigned long, const char *} and the macro would change to
#define FLAGS_DESC(flags) {(flags), (#flags)}

>> @@ -155,8 +169,14 @@ static void do_test(unsigned int number)
>>   		return;
>>   	}
> 
>> -	TST_EXP_FD_OR_FAIL(fanotify_fd = fanotify_init(tc->init_flags, O_RDONLY),
>> -			   !tc->mask && tc->expected_errno ? tc->expected_errno : 0);
> TST_EXP_FD_OR_FAIL was added only to be used by fanotify tests.
> What's wrong with it?

I got a headache trying to figure out when this call was expected to 
pass and when it was expected to fail. The more verbose version below is 
far easier to understand.

>> +	if (!tc->mask && tc->expected_errno) {
>> +		TST_EXP_FAIL(fanotify_init(tc->init_flags, O_RDONLY),
>> +			tc->expected_errno);
>> +	} else {
>> +		TST_EXP_FD(fanotify_init(tc->init_flags, O_RDONLY));
>> +	}
>> +
>> +	fanotify_fd = TST_RET;

-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


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

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

* Re: [LTP] [PATCH 2/4] Move fanotify fallback constants and structs to LAPI header
  2022-10-13 22:08   ` Petr Vorel
@ 2022-10-14 15:56     ` Martin Doucha
  2022-10-14 16:20       ` Petr Vorel
  0 siblings, 1 reply; 14+ messages in thread
From: Martin Doucha @ 2022-10-14 15:56 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

On 14. 10. 22 0:08, Petr Vorel wrote:
> Hi Martin,
> 
> although it will be used only by tests in testcases/kernel/syscalls/fanotify/,
> it's cleaner to follow LTP approach.
> 
> Reviewed-by: Petr Vorel <petr.vorel@gmail.com>
> 
> ...
>> --- /dev/null
>> +++ b/include/lapi/fanotify.h
>> @@ -0,0 +1,210 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Copyright (c) 2012-2020 Linux Test Project.  All Rights Reserved.
> FAN_FS_ERROR has been added in 3db153665 ("syscalls/fanotify22: Introduce FAN_FS_ERROR test")
> which is 2022. Can be changed before merge.

OK, please update the copyright header. This patch doesn't depend on the 
first one so you can merge it right away.

-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


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

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

* Re: [LTP] [PATCH 1/4] fanotify14: Print human-readable test case flags
  2022-10-14 15:49     ` Martin Doucha
@ 2022-10-14 16:17       ` Petr Vorel
  0 siblings, 0 replies; 14+ messages in thread
From: Petr Vorel @ 2022-10-14 16:17 UTC (permalink / raw)
  To: Martin Doucha; +Cc: ltp

> On 13. 10. 22 23:36, Petr Vorel wrote:
> > Hi Martin,

> > > It's hard to tell which test case is failing from the current fanotify14
> > > output. Print test case flags to make failure analysis easier.

> > > Signed-off-by: Martin Doucha <mdoucha@suse.cz>
> > > ---
> > >   .../kernel/syscalls/fanotify/fanotify14.c     | 194 ++++++++++--------
> > >   1 file changed, 108 insertions(+), 86 deletions(-)

> > > diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c
> > > index 594259ccf..ee42aaf68 100644
> > > --- a/testcases/kernel/syscalls/fanotify/fanotify14.c
> > > +++ b/testcases/kernel/syscalls/fanotify/fanotify14.c
> > > @@ -38,6 +38,8 @@
> > >   #define INODE_EVENTS (FAN_ATTRIB | FAN_CREATE | FAN_DELETE | FAN_MOVE | \
> > >   		      FAN_DELETE_SELF | FAN_MOVE_SELF)

> > > +#define FLAGS_DESC(flags) (flags), (#flags)
> > +1 for add ing description. But macro like this gets false positive in make
> > check:

> > fanotify14.c:41: ERROR: Macros with complex values should be enclosed in parentheses

> > Also quite recently, in dbb9db6ec ("syscalls/fanotify09: Make test case
> > definitions more readable") was single test migrated to use
> > .foo = value, .bar = value struct setup. This is about source code readability,
> > you aim for test output readability, IMHO both is important.

> I can't use that approach here because I'm using the macro to initialize 3
> different pairs of attributes in the same structure. What I could do is
> change the flags/desc pairs into a nested struct of
> {unsigned long, const char *} and the macro would change to
> #define FLAGS_DESC(flags) {(flags), (#flags)}

+1, but it's not important. Amir's comments are what is important.

> > > @@ -155,8 +169,14 @@ static void do_test(unsigned int number)
> > >   		return;
> > >   	}

> > > -	TST_EXP_FD_OR_FAIL(fanotify_fd = fanotify_init(tc->init_flags, O_RDONLY),
> > > -			   !tc->mask && tc->expected_errno ? tc->expected_errno : 0);
> > TST_EXP_FD_OR_FAIL was added only to be used by fanotify tests.
> > What's wrong with it?

> I got a headache trying to figure out when this call was expected to pass
> and when it was expected to fail. The more verbose version below is far
> easier to understand.

Sure, thanks for an explanation.

Kind regards,
Petr

> > > +	if (!tc->mask && tc->expected_errno) {
> > > +		TST_EXP_FAIL(fanotify_init(tc->init_flags, O_RDONLY),
> > > +			tc->expected_errno);
> > > +	} else {
> > > +		TST_EXP_FD(fanotify_init(tc->init_flags, O_RDONLY));
> > > +	}
> > > +
> > > +	fanotify_fd = TST_RET;

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

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

* Re: [LTP] [PATCH 2/4] Move fanotify fallback constants and structs to LAPI header
  2022-10-14 15:56     ` Martin Doucha
@ 2022-10-14 16:20       ` Petr Vorel
  0 siblings, 0 replies; 14+ messages in thread
From: Petr Vorel @ 2022-10-14 16:20 UTC (permalink / raw)
  To: Martin Doucha; +Cc: ltp

Hi Martin, all,

> OK, please update the copyright header. This patch doesn't depend on the
> first one so you can merge it right away.

Merged this one, thanks!

Kind regards,
Petr

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

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

end of thread, other threads:[~2022-10-14 16:20 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-13 15:49 [LTP] [PATCH 0/4] Fix fanotify14 Martin Doucha
2022-10-13 15:49 ` [LTP] [PATCH 1/4] fanotify14: Print human-readable test case flags Martin Doucha
2022-10-13 21:36   ` Petr Vorel
2022-10-14 15:49     ` Martin Doucha
2022-10-14 16:17       ` Petr Vorel
2022-10-13 15:49 ` [LTP] [PATCH 2/4] Move fanotify fallback constants and structs to LAPI header Martin Doucha
2022-10-13 22:08   ` Petr Vorel
2022-10-14 15:56     ` Martin Doucha
2022-10-14 16:20       ` Petr Vorel
2022-10-13 15:49 ` [LTP] [PATCH 3/4] Add fanotify_get_supported_init_flags() helper function Martin Doucha
2022-10-14  6:15   ` Petr Vorel
2022-10-14  6:50   ` Amir Goldstein
2022-10-13 15:49 ` [LTP] [PATCH 4/4] fanotify14: Improve check for unsupported init flags Martin Doucha
2022-10-14  6:20   ` Petr Vorel

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