Hello Amir,
On 2/26/19 5:45 PM, Amir Goldstein wrote:
> On Tue, Feb 26, 2019 at 6:31 PM Michael Kerrisk (man-pages)
> <mtk.manpages@xxxxxxxxx> wrote:
>>
>> Hello Amir,
>>
>> On 2/26/19 5:18 PM, Michael Kerrisk (man-pages) wrote:
>>> Hello Amir,
>>>
>>> Thanks! I've applied this patch, but I have a question below.
>>>
>>> On 11/17/18 5:32 PM, Amir Goldstein wrote:
>>>> Monitor fanotify events on the entire filesystem.
>>>>
>>>> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
>>>> ---
>>>> man2/fanotify_mark.2 | 24 +++++++++++++++++++++---
>>>> man7/fanotify.7 | 20 +++++++++++---------
>>>> 2 files changed, 32 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/man2/fanotify_mark.2 b/man2/fanotify_mark.2
>>>> index a9a482fe7..3862a8e34 100644
>>>> --- a/man2/fanotify_mark.2
>>>> +++ b/man2/fanotify_mark.2
>>>> @@ -70,15 +70,23 @@ must be nonempty or the error
>>>> will occur.
>>>> .TP
>>>> .B FAN_MARK_FLUSH
>>>> -Remove either all mount or all non-mount marks from the fanotify group.
>>>> +Remove either all marks for filesystems, all marks for mounts or all
>>>> +marks for directories and files from the fanotify group.
>>>> If
>>>> .I flags
>>>> contains
>>>> .BR FAN_MARK_MOUNT ,
>>>> all marks for mounts are removed from the group.
>>>> +If
>>>> +.I flags
>>>> +contains
>>>> +.BR FAN_MARK_FILESYSTEM ,
>>>> +all marks for filesystems are removed from the group.
>>>> Otherwise, all marks for directories and files are removed.
>>>> -No flag other than
>>>> +No flag other than and only one of the flags
>>>
>>> I think rather than "and only one of", the text should say
>>> "and at most one of", since both of these flags could be omitted,
>>> right?
>
> Correct.
Okay -- I reworded there.
>> Looking at the kernel code more closely, it seems there is no
>> error generated if both FAN_MARK_MOUNT and FAN_MARK_FILESYSTEM
>> are specified in the flags. That seems rather unkind to the user.
>> How about adding an error return in this case, so that the user
>> doesn't get surprised?
>>
>
> Hmm? That should return EINVAL.
> Although I did not write a regression test for this flag validation...
>
> #define FANOTIFY_MARK_TYPE_BITS (FAN_MARK_INODE | FAN_MARK_MOUNT | \
> FAN_MARK_FILESYSTEM)
> ...
> unsigned int mark_type = flags & FANOTIFY_MARK_TYPE_BITS;
> ...
> switch (mark_type) {
> case FAN_MARK_INODE:
> case FAN_MARK_MOUNT:
> case FAN_MARK_FILESYSTEM:
> break;
> default:
> return -EINVAL;
> }
>
> Am I missing something?
No, I was; I overlooked the piece you show above :-}
Thanks,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
