Message ID | 20220907110326.2915779-1-amir73il@gmail.com |
---|---|
State | Accepted |
Headers | show |
Series | syscalls/fanotify09: Make test case definitions more readable | expand |
Hi Amir, > Hi Petr, > Here is the cleanup you proposed. > Please check that I did not make any mistakes... LGTM. Thank you! Reviewed-by: Petr Vorel <pvorel@suse.cz> Kind regards, Petr > Thanks, > Amir.
On Wed, Sep 07, 2022 at 02:03:26PM +0300, Amir Goldstein wrote: > Use explicit field members to initialize test cases and omit > fields initialized to zero to make the definitions more compact > and more readable. Looks fine. I guess we could adopt the same designated intializer pattern across a bunch of other fanotify LTP tests. Reviewed-by: Matthew Bobrowski <repnop@google.com> > Rename the field s/close_nowrite/event_path to make its meaning > less obscure. > > Suggested-by: Petr Vorel <pvorel@suse.cz> > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > --- > > Hi Petr, > > Here is the cleanup you proposed. > Please check that I did not make any mistakes... > > Thanks, > Amir. > > .../kernel/syscalls/fanotify/fanotify09.c | 203 ++++++++---------- > 1 file changed, 94 insertions(+), 109 deletions(-) > > diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c > index e40916c08..3f2db4709 100644 > --- a/testcases/kernel/syscalls/fanotify/fanotify09.c > +++ b/testcases/kernel/syscalls/fanotify/fanotify09.c > @@ -82,145 +82,130 @@ static struct tcase { > unsigned int ignore; > unsigned int ignore_flags; > unsigned int report_name; > - const char *close_nowrite; > + const char *event_path; > int nevents; > unsigned int nonfirst_event; > } tcases[] = { > { > - "Events on non-dir child with both parent and mount marks", > - INIT_FANOTIFY_MARK_TYPE(MOUNT), > - 0, > - 0, 0, > - 0, > - DIR_NAME, > - 1, 0, > + .tname = "Events on non-dir child with both parent and mount marks", > + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), > + .event_path = DIR_NAME, > + .nevents = 1, > }, > { > - "Events on non-dir child and subdir with both parent and mount marks", > - INIT_FANOTIFY_MARK_TYPE(MOUNT), > - FAN_ONDIR, > - 0, 0, > - 0, > - DIR_NAME, > - 2, 0, > + .tname = "Events on non-dir child and subdir with both parent and mount marks", > + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), > + .ondir = FAN_ONDIR, > + .event_path = DIR_NAME, > + .nevents = 2, > }, > { > - "Events on non-dir child and parent with both parent and mount marks", > - INIT_FANOTIFY_MARK_TYPE(MOUNT), > - FAN_ONDIR, > - 0, 0, > - 0, > - ".", > - 2, 0 > + .tname = "Events on non-dir child and parent with both parent and mount marks", > + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), > + .ondir = FAN_ONDIR, > + .event_path = ".", > + .nevents = 2, > }, > { > - "Events on non-dir child and subdir with both parent and subdir marks", > - INIT_FANOTIFY_MARK_TYPE(INODE), > - FAN_ONDIR, > - 0, 0, > - 0, > - DIR_NAME, > - 2, 0, > + .tname = "Events on non-dir child and subdir with both parent and subdir marks", > + .mark = INIT_FANOTIFY_MARK_TYPE(INODE), > + .ondir = FAN_ONDIR, > + .event_path = DIR_NAME, > + .nevents = 2, > }, > { > - "Events on non-dir children with both parent and mount marks", > - INIT_FANOTIFY_MARK_TYPE(MOUNT), > - 0, > - 0, 0, > - 0, > - FILE2_NAME, > - 2, FAN_CLOSE_NOWRITE, > + .tname = "Events on non-dir children with both parent and mount marks", > + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), > + .event_path = FILE2_NAME, > + .nevents = 2, > + .nonfirst_event = FAN_CLOSE_NOWRITE, > }, > { > - "Events on non-dir child with both parent and mount marks and filename info", > - INIT_FANOTIFY_MARK_TYPE(MOUNT), > - 0, > - 0, 0, > - FAN_REPORT_DFID_NAME, > - FILE2_NAME, > - 2, FAN_CLOSE_NOWRITE, > + .tname = "Events on non-dir child with both parent and mount marks and filename info", > + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), > + .report_name = FAN_REPORT_DFID_NAME, > + .event_path = FILE2_NAME, > + .nevents = 2, > + .nonfirst_event = FAN_CLOSE_NOWRITE, > }, > { > - "Events on non-dir child with ignore mask on parent", > - INIT_FANOTIFY_MARK_TYPE(MOUNT), > - 0, > - FAN_MARK_IGNORED_MASK, 0, > - 0, > - DIR_NAME, > - 1, 0, > + .tname = "Events on non-dir child with ignore mask on parent", > + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), > + .ignore = FAN_MARK_IGNORED_MASK, > + .event_path = DIR_NAME, > + .nevents = 1, > }, > { > - "Events on non-dir children with surviving ignore mask on parent", > - INIT_FANOTIFY_MARK_TYPE(MOUNT), > - 0, > - FAN_MARK_IGNORED_MASK | FAN_MARK_IGNORED_SURV_MODIFY, 0, > - 0, > - FILE2_NAME, > - 2, FAN_CLOSE_NOWRITE, > + .tname = "Events on non-dir children with surviving ignore mask on parent", > + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), > + .ignore = FAN_MARK_IGNORED_MASK | FAN_MARK_IGNORED_SURV_MODIFY, > + .event_path = FILE2_NAME, > + .nevents = 2, > + .nonfirst_event = FAN_CLOSE_NOWRITE, > }, > /* FAN_MARK_IGNORE test cases: */ > { > - "Events on dir with ignore mask that does not apply to dirs", > - INIT_FANOTIFY_MARK_TYPE(MOUNT), > - FAN_ONDIR, > - FAN_MARK_IGNORE_SURV, 0, > - 0, > - ".", > - 2, FAN_CLOSE_NOWRITE, > + .tname = "Events on dir with ignore mask that does not apply to dirs", > + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), > + .ondir = FAN_ONDIR, > + .ignore = FAN_MARK_IGNORE_SURV, > + .event_path = ".", > + .nevents = 2, > + .nonfirst_event = FAN_CLOSE_NOWRITE, > }, > { > - "Events on dir with ignore mask that does apply to dirs", > - INIT_FANOTIFY_MARK_TYPE(MOUNT), > - FAN_ONDIR, > - FAN_MARK_IGNORE_SURV, FAN_ONDIR, > - 0, > - ".", > - 2, 0, > + .tname = "Events on dir with ignore mask that does apply to dirs", > + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), > + .ondir = FAN_ONDIR, > + .ignore = FAN_MARK_IGNORE_SURV, > + .ignore_flags = FAN_ONDIR, > + .event_path = ".", > + .nevents = 2, > }, > { > - "Events on child with ignore mask on parent that does not apply to children", > - INIT_FANOTIFY_MARK_TYPE(MOUNT), > - 0, > - FAN_MARK_IGNORE_SURV, 0, > - 0, > - FILE2_NAME, > - 2, FAN_CLOSE_NOWRITE, > + .tname = "Events on child with ignore mask on parent that does not apply to children", > + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), > + .ignore = FAN_MARK_IGNORE_SURV, > + .event_path = FILE2_NAME, > + .nevents = 2, > + .nonfirst_event = FAN_CLOSE_NOWRITE, > }, > { > - "Events on child with ignore mask on parent that does apply to children", > - INIT_FANOTIFY_MARK_TYPE(MOUNT), > - 0, > - FAN_MARK_IGNORE_SURV, FAN_EVENT_ON_CHILD, > - 0, > - FILE2_NAME, > - 2, 0, > + .tname = "Events on child with ignore mask on parent that does apply to children", > + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), > + .ignore = FAN_MARK_IGNORE_SURV, > + .ignore_flags = FAN_EVENT_ON_CHILD, > + .event_path = FILE2_NAME, > + .nevents = 2, > }, > { > - "Events on subdir with ignore mask on parent that does not apply to children", > - INIT_FANOTIFY_MARK_TYPE(MOUNT), > - FAN_ONDIR, > - FAN_MARK_IGNORE_SURV, FAN_ONDIR, > - 0, > - DIR_NAME, > - 2, FAN_CLOSE_NOWRITE, > + .tname = "Events on subdir with ignore mask on parent that does not apply to children", > + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), > + .ondir = FAN_ONDIR, > + .ignore = FAN_MARK_IGNORE_SURV, > + .ignore_flags = FAN_ONDIR, > + .event_path = DIR_NAME, > + .nevents = 2, > + .nonfirst_event = FAN_CLOSE_NOWRITE, > }, > { > - "Events on subdir with ignore mask on parent that does not apply to dirs", > - INIT_FANOTIFY_MARK_TYPE(MOUNT), > - FAN_ONDIR, > - FAN_MARK_IGNORE_SURV, FAN_EVENT_ON_CHILD, > - 0, > - DIR_NAME, > - 2, FAN_CLOSE_NOWRITE, > + .tname = "Events on subdir with ignore mask on parent that does not apply to dirs", > + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), > + .ondir = FAN_ONDIR, > + .ignore = FAN_MARK_IGNORE_SURV, > + .ignore_flags = FAN_EVENT_ON_CHILD, > + .event_path = DIR_NAME, > + .nevents = 2, > + .nonfirst_event = FAN_CLOSE_NOWRITE, > }, > { > - "Events on subdir with ignore mask on parent that does apply to subdirs", > - INIT_FANOTIFY_MARK_TYPE(MOUNT), > - FAN_ONDIR, > - FAN_MARK_IGNORE_SURV, FAN_EVENT_ON_CHILD | FAN_ONDIR, > - 0, > - DIR_NAME, > - 2, 0, > + .tname = "Events on subdir with ignore mask on parent that does apply to subdirs", > + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), > + .ondir = FAN_ONDIR, > + .ignore = FAN_MARK_IGNORE_SURV, > + .ignore_flags = FAN_EVENT_ON_CHILD | FAN_ONDIR, > + .event_path = DIR_NAME, > + .nevents = 2, > }, > }; > > @@ -261,7 +246,7 @@ static void create_fanotify_groups(struct tcase *tc) > SAFE_FANOTIFY_MARK(fd_notify[i], > FAN_MARK_ADD | mark->flag, > FAN_CLOSE_NOWRITE | mask_flags, > - AT_FDCWD, tc->close_nowrite); > + AT_FDCWD, tc->event_path); > > /* > * Add inode mark on parent for each group with MODIFY event, > @@ -415,7 +400,7 @@ static void test_fanotify(unsigned int n) > /* > * generate FAN_CLOSE_NOWRITE event on a child, subdir or "." > */ > - dirfd = SAFE_OPEN(tc->close_nowrite, O_RDONLY); > + dirfd = SAFE_OPEN(tc->event_path, O_RDONLY); > SAFE_CLOSE(dirfd); > > /* > @@ -443,7 +428,7 @@ static void test_fanotify(unsigned int n) > } > if (tc->nevents > 1 && FAN_EVENT_OK(event, ret)) { > verify_event(0, event, FAN_CLOSE_NOWRITE, > - tc->report_name ? (tc->ondir ? "." : tc->close_nowrite) : ""); > + tc->report_name ? (tc->ondir ? "." : tc->event_path) : ""); > event = FAN_EVENT_NEXT(event, ret); > } > if (ret > 0) { > -- > 2.25.1 > /M
Hi all, > On Wed, Sep 07, 2022 at 02:03:26PM +0300, Amir Goldstein wrote: > > Use explicit field members to initialize test cases and omit > > fields initialized to zero to make the definitions more compact > > and more readable. Merged, thank you! > Looks fine. I guess we could adopt the same designated intializer pattern across > a bunch of other fanotify LTP tests. +1 Kind regards, Petr
diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c index e40916c08..3f2db4709 100644 --- a/testcases/kernel/syscalls/fanotify/fanotify09.c +++ b/testcases/kernel/syscalls/fanotify/fanotify09.c @@ -82,145 +82,130 @@ static struct tcase { unsigned int ignore; unsigned int ignore_flags; unsigned int report_name; - const char *close_nowrite; + const char *event_path; int nevents; unsigned int nonfirst_event; } tcases[] = { { - "Events on non-dir child with both parent and mount marks", - INIT_FANOTIFY_MARK_TYPE(MOUNT), - 0, - 0, 0, - 0, - DIR_NAME, - 1, 0, + .tname = "Events on non-dir child with both parent and mount marks", + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), + .event_path = DIR_NAME, + .nevents = 1, }, { - "Events on non-dir child and subdir with both parent and mount marks", - INIT_FANOTIFY_MARK_TYPE(MOUNT), - FAN_ONDIR, - 0, 0, - 0, - DIR_NAME, - 2, 0, + .tname = "Events on non-dir child and subdir with both parent and mount marks", + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), + .ondir = FAN_ONDIR, + .event_path = DIR_NAME, + .nevents = 2, }, { - "Events on non-dir child and parent with both parent and mount marks", - INIT_FANOTIFY_MARK_TYPE(MOUNT), - FAN_ONDIR, - 0, 0, - 0, - ".", - 2, 0 + .tname = "Events on non-dir child and parent with both parent and mount marks", + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), + .ondir = FAN_ONDIR, + .event_path = ".", + .nevents = 2, }, { - "Events on non-dir child and subdir with both parent and subdir marks", - INIT_FANOTIFY_MARK_TYPE(INODE), - FAN_ONDIR, - 0, 0, - 0, - DIR_NAME, - 2, 0, + .tname = "Events on non-dir child and subdir with both parent and subdir marks", + .mark = INIT_FANOTIFY_MARK_TYPE(INODE), + .ondir = FAN_ONDIR, + .event_path = DIR_NAME, + .nevents = 2, }, { - "Events on non-dir children with both parent and mount marks", - INIT_FANOTIFY_MARK_TYPE(MOUNT), - 0, - 0, 0, - 0, - FILE2_NAME, - 2, FAN_CLOSE_NOWRITE, + .tname = "Events on non-dir children with both parent and mount marks", + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), + .event_path = FILE2_NAME, + .nevents = 2, + .nonfirst_event = FAN_CLOSE_NOWRITE, }, { - "Events on non-dir child with both parent and mount marks and filename info", - INIT_FANOTIFY_MARK_TYPE(MOUNT), - 0, - 0, 0, - FAN_REPORT_DFID_NAME, - FILE2_NAME, - 2, FAN_CLOSE_NOWRITE, + .tname = "Events on non-dir child with both parent and mount marks and filename info", + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), + .report_name = FAN_REPORT_DFID_NAME, + .event_path = FILE2_NAME, + .nevents = 2, + .nonfirst_event = FAN_CLOSE_NOWRITE, }, { - "Events on non-dir child with ignore mask on parent", - INIT_FANOTIFY_MARK_TYPE(MOUNT), - 0, - FAN_MARK_IGNORED_MASK, 0, - 0, - DIR_NAME, - 1, 0, + .tname = "Events on non-dir child with ignore mask on parent", + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), + .ignore = FAN_MARK_IGNORED_MASK, + .event_path = DIR_NAME, + .nevents = 1, }, { - "Events on non-dir children with surviving ignore mask on parent", - INIT_FANOTIFY_MARK_TYPE(MOUNT), - 0, - FAN_MARK_IGNORED_MASK | FAN_MARK_IGNORED_SURV_MODIFY, 0, - 0, - FILE2_NAME, - 2, FAN_CLOSE_NOWRITE, + .tname = "Events on non-dir children with surviving ignore mask on parent", + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), + .ignore = FAN_MARK_IGNORED_MASK | FAN_MARK_IGNORED_SURV_MODIFY, + .event_path = FILE2_NAME, + .nevents = 2, + .nonfirst_event = FAN_CLOSE_NOWRITE, }, /* FAN_MARK_IGNORE test cases: */ { - "Events on dir with ignore mask that does not apply to dirs", - INIT_FANOTIFY_MARK_TYPE(MOUNT), - FAN_ONDIR, - FAN_MARK_IGNORE_SURV, 0, - 0, - ".", - 2, FAN_CLOSE_NOWRITE, + .tname = "Events on dir with ignore mask that does not apply to dirs", + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), + .ondir = FAN_ONDIR, + .ignore = FAN_MARK_IGNORE_SURV, + .event_path = ".", + .nevents = 2, + .nonfirst_event = FAN_CLOSE_NOWRITE, }, { - "Events on dir with ignore mask that does apply to dirs", - INIT_FANOTIFY_MARK_TYPE(MOUNT), - FAN_ONDIR, - FAN_MARK_IGNORE_SURV, FAN_ONDIR, - 0, - ".", - 2, 0, + .tname = "Events on dir with ignore mask that does apply to dirs", + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), + .ondir = FAN_ONDIR, + .ignore = FAN_MARK_IGNORE_SURV, + .ignore_flags = FAN_ONDIR, + .event_path = ".", + .nevents = 2, }, { - "Events on child with ignore mask on parent that does not apply to children", - INIT_FANOTIFY_MARK_TYPE(MOUNT), - 0, - FAN_MARK_IGNORE_SURV, 0, - 0, - FILE2_NAME, - 2, FAN_CLOSE_NOWRITE, + .tname = "Events on child with ignore mask on parent that does not apply to children", + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), + .ignore = FAN_MARK_IGNORE_SURV, + .event_path = FILE2_NAME, + .nevents = 2, + .nonfirst_event = FAN_CLOSE_NOWRITE, }, { - "Events on child with ignore mask on parent that does apply to children", - INIT_FANOTIFY_MARK_TYPE(MOUNT), - 0, - FAN_MARK_IGNORE_SURV, FAN_EVENT_ON_CHILD, - 0, - FILE2_NAME, - 2, 0, + .tname = "Events on child with ignore mask on parent that does apply to children", + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), + .ignore = FAN_MARK_IGNORE_SURV, + .ignore_flags = FAN_EVENT_ON_CHILD, + .event_path = FILE2_NAME, + .nevents = 2, }, { - "Events on subdir with ignore mask on parent that does not apply to children", - INIT_FANOTIFY_MARK_TYPE(MOUNT), - FAN_ONDIR, - FAN_MARK_IGNORE_SURV, FAN_ONDIR, - 0, - DIR_NAME, - 2, FAN_CLOSE_NOWRITE, + .tname = "Events on subdir with ignore mask on parent that does not apply to children", + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), + .ondir = FAN_ONDIR, + .ignore = FAN_MARK_IGNORE_SURV, + .ignore_flags = FAN_ONDIR, + .event_path = DIR_NAME, + .nevents = 2, + .nonfirst_event = FAN_CLOSE_NOWRITE, }, { - "Events on subdir with ignore mask on parent that does not apply to dirs", - INIT_FANOTIFY_MARK_TYPE(MOUNT), - FAN_ONDIR, - FAN_MARK_IGNORE_SURV, FAN_EVENT_ON_CHILD, - 0, - DIR_NAME, - 2, FAN_CLOSE_NOWRITE, + .tname = "Events on subdir with ignore mask on parent that does not apply to dirs", + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), + .ondir = FAN_ONDIR, + .ignore = FAN_MARK_IGNORE_SURV, + .ignore_flags = FAN_EVENT_ON_CHILD, + .event_path = DIR_NAME, + .nevents = 2, + .nonfirst_event = FAN_CLOSE_NOWRITE, }, { - "Events on subdir with ignore mask on parent that does apply to subdirs", - INIT_FANOTIFY_MARK_TYPE(MOUNT), - FAN_ONDIR, - FAN_MARK_IGNORE_SURV, FAN_EVENT_ON_CHILD | FAN_ONDIR, - 0, - DIR_NAME, - 2, 0, + .tname = "Events on subdir with ignore mask on parent that does apply to subdirs", + .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), + .ondir = FAN_ONDIR, + .ignore = FAN_MARK_IGNORE_SURV, + .ignore_flags = FAN_EVENT_ON_CHILD | FAN_ONDIR, + .event_path = DIR_NAME, + .nevents = 2, }, }; @@ -261,7 +246,7 @@ static void create_fanotify_groups(struct tcase *tc) SAFE_FANOTIFY_MARK(fd_notify[i], FAN_MARK_ADD | mark->flag, FAN_CLOSE_NOWRITE | mask_flags, - AT_FDCWD, tc->close_nowrite); + AT_FDCWD, tc->event_path); /* * Add inode mark on parent for each group with MODIFY event, @@ -415,7 +400,7 @@ static void test_fanotify(unsigned int n) /* * generate FAN_CLOSE_NOWRITE event on a child, subdir or "." */ - dirfd = SAFE_OPEN(tc->close_nowrite, O_RDONLY); + dirfd = SAFE_OPEN(tc->event_path, O_RDONLY); SAFE_CLOSE(dirfd); /* @@ -443,7 +428,7 @@ static void test_fanotify(unsigned int n) } if (tc->nevents > 1 && FAN_EVENT_OK(event, ret)) { verify_event(0, event, FAN_CLOSE_NOWRITE, - tc->report_name ? (tc->ondir ? "." : tc->close_nowrite) : ""); + tc->report_name ? (tc->ondir ? "." : tc->event_path) : ""); event = FAN_EVENT_NEXT(event, ret); } if (ret > 0) {
Use explicit field members to initialize test cases and omit fields initialized to zero to make the definitions more compact and more readable. Rename the field s/close_nowrite/event_path to make its meaning less obscure. Suggested-by: Petr Vorel <pvorel@suse.cz> Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- Hi Petr, Here is the cleanup you proposed. Please check that I did not make any mistakes... Thanks, Amir. .../kernel/syscalls/fanotify/fanotify09.c | 203 ++++++++---------- 1 file changed, 94 insertions(+), 109 deletions(-)