Re: [LTP] [PATCH v2 1/2] syscalls/fanotify20: add new test for FAN_REPORT_PIDFD

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

On Tue, Nov 02, 2021 at 02:15:51PM +0200, Amir Goldstein wrote:
> On Tue, Nov 2, 2021 at 1:16 PM Matthew Bobrowski <repnop@google.com> wrote:
> >
> > On Tue, Nov 02, 2021 at 01:02:48PM +0200, Amir Goldstein wrote:
> > > On Tue, Nov 2, 2021 at 12:57 PM Matthew Bobrowski <repnop@google.com> wrote:
> > > >
> > > > This test ensures that the fanotify API returns the expected error
> > > > status code -EINVAL when an invalid flag is supplied alongside the new
> > > > FAN_REPORT_PIDFD initialization flag. Currently, FAN_REPORT_TID is the
> > > > only initialization flag that is not permitted in conjunction with
> > > > FAN_REPORT_PIDFD, so we explicitly provide test coverage for this.
> > > >
> > > > We also add an extra trivial test case to ensure that the
> > > > initialization behavior with the other FAN_REPORT_* related flags is
> > > > working as intended.
> > > >
> > > > Signed-off-by: Matthew Bobrowski <repnop@google.com>
> > > > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > > > ---
> > > > Changes since v1:
> > > >  - Introduced a new macro
> > > >    REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_BY_KERNEL() that is
> > > >    responsible for testing whether the supplied initialization flags
> > > >    are supported by the underlying kernel. This is used from
> > > >    do_setup(). Using this is less ambiguous then using something like
> > > >    REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS().
> > >
> > > Not like this.
> > > Please start your branch with the first two prep patches from
> > > Gabriel's LTP post (including my reviewed-by tag) preserving
> > > Gabriel's authorship and signed-of-by and adding your own
> > > signed-off-by.
> > >
> > > Your LTP tests are expected to be merged before Gabriel's test
> > > because your tests are for a 5.15 feature.
> > > Once your tests are merge, Gabriel would be able to rebase on master
> > > and drop his prep patches.
> >
> > Am I reading all the words, or only some of the words?
> >
> > AFAICT, the macro that I've defined here is different to that of what
> > has been implemented in Gabriel's series.
> >
> 
> My bad.
> I wasn't paying attention to the difference.
> It wouldn't hurt to split the macro patch anyway.

Right, I'll split and repost the series.

Thanks for the review!

/M

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