On Thu, Feb 13, 2025 at 1:00 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
>
> On Tue, 11 Feb 2025 at 16:50, Jan Kara <jack@xxxxxxx> wrote:
> >
> > On Wed 29-01-25 17:58:00, Miklos Szeredi wrote:
>
> > > fid_mode = FAN_GROUP_FLAG(group, FANOTIFY_FID_BITS);
> > > - if (mask & ~(FANOTIFY_FD_EVENTS|FANOTIFY_EVENT_FLAGS) &&
> > > + if (mask & ~(FANOTIFY_FD_EVENTS|FANOTIFY_MOUNT_EVENTS|FANOTIFY_EVENT_FLAGS) &&
> >
> > I understand why you need this but the condition is really hard to
> > understand now and the comment above it becomes out of date. Perhaps I'd
> > move this and the following two checks for FAN_RENAME and
> > FANOTIFY_PRE_CONTENT_EVENTS into !FAN_GROUP_FLAG(group, FAN_REPORT_MNT)
> > branch to make things more obvious?
>
> Okay. git diff -w below.
>
> Thanks,
> Miklos
>
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -1936,6 +1936,8 @@ static int do_fanotify_mark(int fanotify_fd,
> unsigned int flags, __u64 mask,
> mark_type != FAN_MARK_INODE)
> return -EINVAL;
>
> + /* The following checks are not relevant to mount events */
> + if (!FAN_GROUP_FLAG(group, FAN_REPORT_MNT)) {
Sorry for nit picking, but you already have this !FAN_REPORT_MNT
branch above:
+ /* Only report mount events on mnt namespace */
+ if (FAN_GROUP_FLAG(group, FAN_REPORT_MNT)) {
+ if (mask & ~FANOTIFY_MOUNT_EVENTS)
+ return -EINVAL;
...
+ } else {
+ if (mask & FANOTIFY_MOUNT_EVENTS)
Which can be easily moved down here and then we get in one place:
if (FAN_REPORT_MNT) {
/* event rules for FAN_REPORT_MNT */
} else {
/* event rules for !FAN_REPORT_MNT */
}
TBH, with the check for (mask & ~FANOTIFY_MOUNT_EVENTS)
I personally wouldn't mind leaving checks for FAN_RENAME and
FANOTIFY_PRE_CONTENT_EVENTS outside of the else branch,
but I don't have a strong objection to including them in the else branch.
Thanks,
Amir.
