Re: [LTP] [PATCH 1/9] syscalls/fanotify09: Add test cases for FAN_MARK_IGNORE

[Petr Vorel <pvorel@suse.cz>]

> On Mon, Sep 5, 2022 at 7:04 PM Petr Vorel <pvorel@suse.cz> wrote:

> > Hi Amir,

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

> > ...
> > > diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
> > > index 0eb83e2f8..e40916c08 100644
> > > --- a/testcases/kernel/syscalls/fanotify/fanotify09.c
> > > +++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
> > > @@ -73,12 +73,14 @@ static char event_buf[EVENT_BUF_LEN];
> > >  static int mount_created;

> > >  static int fan_report_dfid_unsupported;
> > > +static int ignore_mark_unsupported;

> > >  static struct tcase {
> > >       const char *tname;
> > >       struct fanotify_mark_type mark;
> > >       unsigned int ondir;
> > >       unsigned int ignore;
> > > +     unsigned int ignore_flags;
> > >       unsigned int report_name;
> > >       const char *close_nowrite;
> > >       int nevents;
> > > @@ -88,7 +90,7 @@ static struct tcase {
> > >               "Events on non-dir child with both parent and mount marks",
> > >               INIT_FANOTIFY_MARK_TYPE(MOUNT),
> > >               0,
> > > -             0,
> > > +             0, 0,
> > >               0,
> > >               DIR_NAME,
> > >               1, 0,

> > nit: as number of struct grow, it'd help readability to omit members with 0:
> >                 .tname = "Events on non-dir child with both parent and mount marks",
> >                 .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT),
> >                 .close_nowrite = DIR_NAME,
> >                 .nevents = 1,


> You are absolutely right.
> The readability of use cases is quite tough at this point.

> > This is obviously non-blocker of this patch, if you agree it can be done any
> > time later.

> Certainly.
> I think that growing the struct is a good place to stop and
> do this cleanup, we have got plenty of time until v6.0 and I expect to
> get more comments on this series, so I will probably do this cleanup
> already for v2.
Thank you!

FYI we have LTP releases regularly 4x a year [1], the next one will be around
the end of September, with git freeze ~ 2 weeks before, i.e. very soon.
Of course this patchset should get in the release (I don't expect any issues any
major problems reported by Jan or Matthew, checkpatch.pl issues will not block it).

> > Also I fixed make check warnings some time ago (i.e. checkpatch.pl), but they

> Cool. I didn't know.

> > got back. I might find a time to fix them (we appreciate you look after the
> > test, thus don't want to bother you with these tiny details).


> This is the only new check warning I see:

> fanotify10.c:378: ERROR: Bad function definition - void drop_caches()
> should probably be void drop_caches(void)
> make: [../../../../include/mk/rules.mk:56: check-fanotify10] Error 1 (ignored)
> fanotify10.c:378:25: warning: non-ANSI function declaration of
> function 'drop_caches'
My bad - I fixed it before 0cb281815 ("tools: Check headers with checkpatch.pl")
which included the report also for headers. And yes, the rest of (mostly)
formatting reports are in fanotify.h.

Obviously we haven't found all checks which aren't relevant for userspace,
therefore I posted patch ignore some of them in LTP [2].

> I will fix it.
Thank you.

Kind regards,
Petr

[1] https://github.com/linux-test-project/ltp/releases
[2] https://lore.kernel.org/ltp/20220906054612.9790-1-pvorel@suse.cz/

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