[LTP] [PATCH v5 04/10] fanotify: Add helper for FAN_REPORT_FID support on fs

Amir Goldstein
2020-12-04T10:11:35Z

On Wed, Dec 2, 2020 at 8:04 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Amir, Cyril,
>
> > > + if (fanotify_mark(fd, FAN_MARK_ADD, FAN_ACCESS, AT_FDCWD, fname) < 0) {
> > > + if (errno == ENODEV || errno == EOPNOTSUPP || errno == EXDEV) {
> > > + rval = -2;
> > > + } else {
> > > + tst_brk(TBROK | TERRNO,
> > > + "fanotify_mark (%d, FAN_MARK_ADD, ..., AT_FDCWD, \".\") failed", fd);
> > ^
> > fname?
> ...
>
> > Maybe this would be just easier to read as a macro:
>
> > #define FANOTIFY_FAN_REPORT_FID_ERR_MSG(res_func, fail) do { \
> > if (fail == -1) \
> > res_func(TCONF, "FAN_REPORT_FID not supported in kernel?"); \
> > if (fail == -2) \
> > res_func(TCONF, ...); \
> > } while (0)
>

Sorry, I ended up reverting that back to a function.

> ...
> > > +static inline void require_fanotify_fan_report_fid_supported_on_fs(const char *file,
> > > + const int lineno, const char *fname)
> > > +{
> > > + int rval;
> > > +
> > > + rval = fanotify_fan_report_fid_supported_on_fs(fname);
> > > + if (rval)
> > > + fanotify_fan_report_fid_err_msg(file, lineno, tst_brk_, rval);
>
> > We don't have to do the if here, just pass the rval, it will not trigger
> > tst_brk() if we pass zero.
>
>
> > The rest is good.
>
> Both your comments fixed, patchset merged.
>
> Thank you both for your patient review,
>
> Amir, I'm sorry it took me that long.

No worries.
I know there was a lot of black magic behind all the flag checks
that needed explanations.

> Looking forward to your patchset :).
>

Posted.
Note that I only tested upstream, v5.8, v4.20 and not kernels
without permission events support and not exfat.

I trust you will help to fill those testing gaps.

If I am not mistaken, before my fixes, fanotify10 was going to fail
instead of TCONF on exfat/ntfs and kernel >= v5.9, but I did not check.

Thanks,
Amir.