On Wed 28-11-18 20:34:47, Amir Goldstein wrote:
> On Wed, Nov 28, 2018 at 7:43 PM Jan Kara <jack@xxxxxxx> wrote:
> >
> > On Wed 28-11-18 18:24:11, Amir Goldstein wrote:
> > > On Wed, Nov 28, 2018 at 5:27 PM Jan Kara <jack@xxxxxxx> wrote:
> > > > > diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> > > > > index 2dbb2662a92f..93e1aa2a389f 100644
> > > > > --- a/fs/notify/fanotify/fanotify_user.c
> > > > > +++ b/fs/notify/fanotify/fanotify_user.c
> > > > > @@ -133,9 +133,10 @@ static int fill_event_metadata(struct fsnotify_group *group,
> > > > > metadata->reserved = 0;
> > > > > metadata->mask = fsn_event->mask & FANOTIFY_OUTGOING_EVENTS;
> > > > > metadata->pid = pid_vnr(event->pid);
> > > > > - if (unlikely(fsn_event->mask & FAN_Q_OVERFLOW))
> > > > > + if (FAN_GROUP_FLAG(group, FAN_REPORT_FID) ||
> > > > > + unlikely(fsn_event->mask & FAN_Q_OVERFLOW)) {
> > > > > metadata->fd = FAN_NOFD;
> > > > > - else {
> > > > > + } else {
> > > >
> > > > Use FANOTIFY_HAS_FID() helper here?
> > >
> > > Not here. FAN_REPORT_FID implies that event->fd will never be used.
> > > But I need to check FANOTIFY_HAS_FID() before copy_fid_to_user(),
> > > because we could fail to decode fid and still report the event without fid.
> >
> > OK. So maybe something like would be more obvious?
> >
> > if (fanotify_event_has_path(event)) {
> > create and store fd
> > } else if (fanotify_event_has_fid(event))
> > store fid
> > } else {
> > clear fd & fid
> > }
>
> The issue is that fill_event_metadata() function fills a fixed size header
> and later copy_event_to_user() copies that header to user and then
> copies the variable sized fid to user, so this is not the place to "store"
> fid, but I will work on readability of this code.
Aha, I see. Thanks for your patience with me :). So then how about just
folding fill_event_metadata() into copy_event_to_user() (as a preparatory
patch). It is relatively small function, has a single call site and with
FID changes the distinction isn't so clear anymore...
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
