Re: [PATCH][RFC] fanotify: deprecate uapi FAN_ALL_* constants

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue 02-10-18 19:23:53, Amir Goldstein wrote:
> On Tue, Oct 2, 2018 at 6:56 PM Jan Kara <jack@xxxxxxx> wrote:
> > Regarding to this patch I have just two nits:
> >
> > > diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> > > index 94b52157bf8d..e5a3c69848e4 100644
> > > --- a/fs/notify/fanotify/fanotify.c
> > > +++ b/fs/notify/fanotify/fanotify.c
> > > @@ -131,8 +131,8 @@ static bool fanotify_should_send_event(struct fsnotify_iter_info *iter_info,
> > >           !(marks_mask & FS_ISDIR & ~marks_ignored_mask))
> > >               return false;
> > >
> > > -     if (event_mask & FAN_ALL_OUTGOING_EVENTS & marks_mask &
> > > -                              ~marks_ignored_mask)
> > > +     if (event_mask & FAN_USER_OUTGOING_EVENTS &
> > > +         marks_mask & ~marks_ignored_mask)
> > >               return true;
> >
> > I don't like the _USER_ part of the constant name. How about _KNOWN_?
> > I.e., FAN_KNOWN_OUTGOING_EVENTS sounds about like what it should?
> >
> 
> Do you mean just for *this* constant? I would rather have some uniformity
> across constants if possible.

No, I meant for all. Sorry for the confusion.

> Errr... Maybe FAN_VALID_EVENTS/FAN_VALID_OUTGOING_EVENTS
> Although _VALID_ is more appropriate for user inputs, so maybe just
> FAN_KNOWN_OUTGOING_EVENTS as an exception to the convention
> is fine.
> 
> Or simply FAN_EVENTS_MASK/FAN_OUTGOING_EVENTS_MASK, like
> the recent new constant FAN_MARK_TYPE_MASK?

Yeah, MASK is fine with me as well. So are the names with VALID. Just pick
one you like the most.

> > > diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h
> > > index 096c96f4f16a..a67430811006 100644
> > > --- a/include/linux/fanotify.h
> > > +++ b/include/linux/fanotify.h
> > > @@ -4,6 +4,56 @@
> > >
> > >  #include <uapi/linux/fanotify.h>
> > >
> > > -/* not valid from userspace, only kernel internal */
> > > -#define FAN_MARK_ONDIR               0x00000100
> > > +/*
> > > + * Flags not valid from userspace, only kernel internal.
> > > + * Use high bits so we won't collide with userspace flags.
> > > + */
> > > +#define FAN_MARK_ONDIR               0x80000000
> >
> > This ought to be a separate change as I wrote above.
> >
> 
> Sure and maybe I'll name it FAN_KERN_MARK_ONDIR.

Fine by me.

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux