| Message ID | 20220719095853.3373732-2-amir73il@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fanotify tests for v5.19-rc5 | expand |
On Tue, Jul 19, 2022 at 11:58:52AM +0200, Amir Goldstein wrote: > So we can add test cases for errors other than EINVAL. Just one optional nit. We can probably remove the comments which are directly above the existing `if (errno == EINVAL)` checks as they're specific to one expected errno value, but this is no longer the case with ENOTDIR now in some fanotify_init/fanotify_mark cases. Feel free to add RVB tags. > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > --- > .../kernel/syscalls/fanotify/fanotify14.c | 31 ++++++++++--------- > 1 file changed, 16 insertions(+), 15 deletions(-) > > diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c > index 5d74b9b91..c99e19706 100644 > --- a/testcases/kernel/syscalls/fanotify/fanotify14.c > +++ b/testcases/kernel/syscalls/fanotify/fanotify14.c > @@ -41,38 +41,39 @@ static struct test_case_t { > unsigned int init_flags; > unsigned int mark_flags; > unsigned long long mask; > + int expected_errno; > } test_cases[] = { > { > - FAN_CLASS_CONTENT | FAN_REPORT_FID, 0, 0 > + FAN_CLASS_CONTENT | FAN_REPORT_FID, 0, 0, EINVAL > }, > { > - FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID, 0, 0 > + FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID, 0, 0, EINVAL > }, > { > - FAN_CLASS_NOTIF, 0, INODE_EVENTS > + FAN_CLASS_NOTIF, 0, INODE_EVENTS, EINVAL > }, > { > - FAN_CLASS_NOTIF | FAN_REPORT_FID, FAN_MARK_MOUNT, INODE_EVENTS > + 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 > + 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 > + 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 > + 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 > + 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 > + FAN_CLASS_NOTIF | FAN_REPORT_DFID_FID, 0, FAN_RENAME, EINVAL > }, > }; > > @@ -88,12 +89,12 @@ static void do_test(unsigned int number) > * an invalid notification class is specified in > * conjunction with FAN_REPORT_FID. > */ > - if (errno == EINVAL) { > + if (errno == tc->expected_errno) { > tst_res(TPASS, > "fanotify_fd=%d, fanotify_init(%x, O_RDONLY) " > - "failed with error EINVAL as expected", > + "failed with error %d as expected", > fanotify_fd, > - tc->init_flags); > + tc->init_flags, tc->expected_errno); > return; > } > tst_brk(TBROK | TERRNO, > @@ -126,16 +127,16 @@ static void do_test(unsigned int number) > * specified on the notification group, or using > * INODE_EVENTS with mark type FAN_MARK_MOUNT. > */ > - if (errno == EINVAL) { > + if (errno == tc->expected_errno) { > tst_res(TPASS, > "ret=%d, fanotify_mark(%d, FAN_MARK_ADD | %x, " > - "%llx, AT_FDCWD, %s) failed with error EINVAL " > + "%llx, AT_FDCWD, %s) failed with error %d " > "as expected", > ret, > fanotify_fd, > tc->mark_flags, > tc->mask, > - FILE1); > + FILE1, tc->expected_errno); > goto out; > } > tst_brk(TBROK | TERRNO, > -- > 2.25.1 > /M
On Tue, Jul 19, 2022 at 11:58 PM Matthew Bobrowski <repnop@google.com> wrote: > > On Tue, Jul 19, 2022 at 11:58:52AM +0200, Amir Goldstein wrote: > > So we can add test cases for errors other than EINVAL. > > Just one optional nit. We can probably remove the comments which are > directly above the existing `if (errno == EINVAL)` checks as they're > specific to one expected errno value, but this is no longer the case > with ENOTDIR now in some fanotify_init/fanotify_mark cases. Good point. > > Feel free to add RVB tags. Thanks, Amir.
> On Tue, Jul 19, 2022 at 11:58:52AM +0200, Amir Goldstein wrote: > > So we can add test cases for errors other than EINVAL. > Just one optional nit. We can probably remove the comments which are > directly above the existing `if (errno == EINVAL)` checks as they're > specific to one expected errno value, but this is no longer the case > with ENOTDIR now in some fanotify_init/fanotify_mark cases. @Amir, I can remove it in this commit before merge (see diff below), but don't you want to keep this info somewhere? Or feel free to post new version. BTW I tested both commits on various kernels and it works as expected (failing only on 5.17.3 (openSUSE) 5.18.5, (Debian), old kernels TCONF as expected. Reviewed-by: Petr Vorel <pvorel@suse.cz> Tested-by: Petr Vorel <pvorel@suse.cz> > Feel free to add RVB tags. @Matthew FYI adding it with correct form in the email reply help us maintainers to get it added automatically via LTP patchwork instance. Kind regards, Petr diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c index c99e19706..ce7e4c54c 100644 --- a/testcases/kernel/syscalls/fanotify/fanotify14.c +++ b/testcases/kernel/syscalls/fanotify/fanotify14.c @@ -84,11 +84,6 @@ static void do_test(unsigned int number) fanotify_fd = fanotify_init(tc->init_flags, O_RDONLY); if (fanotify_fd < 0) { - /* - * EINVAL is to be returned to the calling process when - * an invalid notification class is specified in - * conjunction with FAN_REPORT_FID. - */ if (errno == tc->expected_errno) { tst_res(TPASS, "fanotify_fd=%d, fanotify_init(%x, O_RDONLY) " @@ -121,12 +116,6 @@ static void do_test(unsigned int number) ret = fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark_flags, tc->mask, AT_FDCWD, FILE1); if (ret < 0) { - /* - * EINVAL is to be returned to the calling process when - * attempting to use INODE_EVENTS without FAN_REPORT_FID - * specified on the notification group, or using - * INODE_EVENTS with mark type FAN_MARK_MOUNT. - */ if (errno == tc->expected_errno) { tst_res(TPASS, "ret=%d, fanotify_mark(%d, FAN_MARK_ADD | %x, "
On Mon, Jul 25, 2022 at 2:49 PM Petr Vorel <pvorel@suse.cz> wrote: > > > On Tue, Jul 19, 2022 at 11:58:52AM +0200, Amir Goldstein wrote: > > > So we can add test cases for errors other than EINVAL. > > > Just one optional nit. We can probably remove the comments which are > > directly above the existing `if (errno == EINVAL)` checks as they're > > specific to one expected errno value, but this is no longer the case > > with ENOTDIR now in some fanotify_init/fanotify_mark cases. > > @Amir, I can remove it in this commit before merge (see diff below), > but don't you want to keep this info somewhere? > Or feel free to post new version. Good idea. I will add the comments to the individual test cases and re-post. Thanks, Amir.
diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c index 5d74b9b91..c99e19706 100644 --- a/testcases/kernel/syscalls/fanotify/fanotify14.c +++ b/testcases/kernel/syscalls/fanotify/fanotify14.c @@ -41,38 +41,39 @@ static struct test_case_t { unsigned int init_flags; unsigned int mark_flags; unsigned long long mask; + int expected_errno; } test_cases[] = { { - FAN_CLASS_CONTENT | FAN_REPORT_FID, 0, 0 + FAN_CLASS_CONTENT | FAN_REPORT_FID, 0, 0, EINVAL }, { - FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID, 0, 0 + FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID, 0, 0, EINVAL }, { - FAN_CLASS_NOTIF, 0, INODE_EVENTS + FAN_CLASS_NOTIF, 0, INODE_EVENTS, EINVAL }, { - FAN_CLASS_NOTIF | FAN_REPORT_FID, FAN_MARK_MOUNT, INODE_EVENTS + 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 + 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 + 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 + 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 + 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 + FAN_CLASS_NOTIF | FAN_REPORT_DFID_FID, 0, FAN_RENAME, EINVAL }, }; @@ -88,12 +89,12 @@ static void do_test(unsigned int number) * an invalid notification class is specified in * conjunction with FAN_REPORT_FID. */ - if (errno == EINVAL) { + if (errno == tc->expected_errno) { tst_res(TPASS, "fanotify_fd=%d, fanotify_init(%x, O_RDONLY) " - "failed with error EINVAL as expected", + "failed with error %d as expected", fanotify_fd, - tc->init_flags); + tc->init_flags, tc->expected_errno); return; } tst_brk(TBROK | TERRNO, @@ -126,16 +127,16 @@ static void do_test(unsigned int number) * specified on the notification group, or using * INODE_EVENTS with mark type FAN_MARK_MOUNT. */ - if (errno == EINVAL) { + if (errno == tc->expected_errno) { tst_res(TPASS, "ret=%d, fanotify_mark(%d, FAN_MARK_ADD | %x, " - "%llx, AT_FDCWD, %s) failed with error EINVAL " + "%llx, AT_FDCWD, %s) failed with error %d " "as expected", ret, fanotify_fd, tc->mark_flags, tc->mask, - FILE1); + FILE1, tc->expected_errno); goto out; } tst_brk(TBROK | TERRNO,
So we can add test cases for errors other than EINVAL. Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- .../kernel/syscalls/fanotify/fanotify14.c | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-)