Re: [PATCH v3 07/13] fanotify: copy event fid info to user

[Jan Kara <jack@suse.cz>]

On Thu 29-11-18 11:27:08, Amir Goldstein wrote:
> On Thu, Nov 29, 2018, 11:00 AM Jan Kara <jack@suse.cz wrote:
> 
> > On Sun 25-11-18 15:43:46, Amir Goldstein wrote:
> > > If group requested FAN_REPORT_FID and event has file identifier
> > > copy that information to user reading the event after reading
> > > event metadata.
> > > metadata->event_len includes the length of the fid information.
> > >
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ...
> > > +static int round_event_fid_len(struct fsnotify_event *fsn_event)
> > > +{
> > > +     struct fanotify_event *event = FANOTIFY_E(fsn_event);
> > > +
> > > +     if (!FANOTIFY_HAS_FID(event))
> > > +             return 0;
> > > +
> > > +     return roundup(FANOTIFY_FID_INFO_LEN(event),
> > FAN_EVENT_METADATA_LEN);
> > > +}
> > > +
> >
> > Why do you round up to FAN_EVENT_METADATA_LEN? I think rounding up to
> > multiple of 8 bytes should be more than enough... Otherwise the patch looks
> > good to me.
> >
> 
> Sorry for html reply . You may reply to list.
> 
> I was following the practice in inotify with variable size events.
> 
> I recon the reasoning is that for simplicity application define an array of
> struct metadata
> And rely on events being metadata size alligned.
> 
> Not sure if that justifies the inotify practice?

Yeah, I don't think following inotify practice is really warranted here.
But thanks for explanation.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR