Re: [RFC PATCH 1/1] fanotify.7, fanotify_init.2, fanotify_mark.2: Document FAN_REPORT_FID and directory modification events

From: Amir Goldstein <amir73il@gmail.com>
To: Jan Kara <jack@suse.cz>
Cc: Matthew Bobrowski <mbobrowski@mbobrowski.org>,
	"Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>,
	linux-man@vger.kernel.org, linux-api@vger.kernel.org,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [RFC PATCH 1/1] fanotify.7, fanotify_init.2, fanotify_mark.2: Document FAN_REPORT_FID and directory modification events
Date: Mon, 18 Mar 2019 13:05:13 +0200	[thread overview]
Message-ID: <CAOQ4uxgEnxp0cHA=pPJK24FW=Y_Rv5gLRN0U6V1nsuvoT5xrxw@mail.gmail.com> (raw)
In-Reply-To: <20190318100138.GB9415@quack2.suse.cz>

> > > > +For example, when an fanotify file descriptor is created using
> > > > +.B FAN_REPORT_FID
> > > > +the
> > > > +.I info_type
> > > > +field of this header is set to
> > > > +.BR FAN_EVENT_INFO_TYPE_FID .
> > > > +Event listeners can use this field to check that events of the correct
> > > > +type are being received.
> > >
> > > I think here you should also speak about the 'len' field in the header
> > > which can be used to skip additional information that is not understood /
> > > uninteresting for the receiver of the event.
> >
> > I don't have any objections to add this as I do believe mentioning it has
> > added benefit. Perhaps we could consider adding in a section which describes
> > all the fanotify_event_info_header fields? Maybe it's not necessary though,
> > and just mentioning these two fields here would be enough. Thoughts?
>
> Well, there are just two fields - 'info_type' and 'len'. There's also 'pad'
> but that doesn't really count ;). So I think that briefly explaining 'len'
> is all that's needed. If you want to formalize this a bit more in a
> separate section, I'm not opposed but I don't think it's really necessary.
>

I did not get a reply from Matthew, so to make sure my comment
about 'len' wasn't missed.

I think 'len' field should get a similar treatment to 'event_len' -
"In the current implementation, the value of len is always
(event_len -  FAN_EVENT_METADATA_LEN). However, the API
is designed..."

And the same sentence about 'event_len' field is now no longer
correct so it should be updated, e.g.:

event_len
      ...
      Without FAN_REPORT_FID, the value of event_len is always
      FAN_EVENT_METADATA_LEN.  With FAN_REPORT_FID,
      event_len also includes the variable length file identifier.

Thanks,
Amir.