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

[Matthew Bobrowski via ltp <ltp@lists.linux.it>]

On Tue, Sep 06, 2022 at 11:26:15AM +0200, Petr Vorel wrote:
> That greatly simplifies the code.

This looks OK to me, nice work!

Reviewed-by: Matthew Bobrowski <repnop@google.com>

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

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