All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Vorel <pvorel@suse.cz>
To: Martin Doucha <mdoucha@suse.cz>
Cc: ltp@lists.linux.it
Subject: Re: [LTP] [PATCH 1/4] fanotify14: Print human-readable test case flags
Date: Thu, 13 Oct 2022 23:36:29 +0200	[thread overview]
Message-ID: <Y0iE3bfMBC56Hi8r@pevik> (raw)
In-Reply-To: <20221013154935.20461-2-mdoucha@suse.cz>

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

  reply	other threads:[~2022-10-13 21:36 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y0iE3bfMBC56Hi8r@pevik \
    --to=pvorel@suse.cz \
    --cc=ltp@lists.linux.it \
    --cc=mdoucha@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.