Michael Albinus <michael.albinus@gmx.de> writes:

> Andreas Politz <politza@hochschule-trier.de> writes:
>> + further simplify filenotify.el nor
>>
>> + handle differing masks in inotify.c .
>
> Thanks for this. Next time you provide a patch, could you pls merge
> emacs recent changes from master first? Your patch was rejected partly,
> and I had to apply rejected hunks manually. Comments:

Sorry, I forgot to pull.

Anyway, here is the more progressive version of the patch, adding both
of the above points.  (I guess, I'm to conservative sometimes and/or
seeing only problems everywhere.)

>> +;; * "inotify_add_watch adds a new watch, or modifies an existing watch"
>> +;;   We need to make sure that different watches for the same directory
>> +;;   don't set the mask in a conflicting way regarding changed/attribute-changed
>> +;; * Also check which other inotify flags are problematic
>> +;;   for concurrent use of the underlying descriptor
>
> Well, I had always the hope to modify inotify watches in this case. If
> there is a watch with flags f1, and a new watch for the same file is
> requested with flags f2, and f2 contains a flag which is not part of f1,
> then either the existing watch shall be adapted, or the existing watch
> shall be removed, and a new shall be installed. Don't know what's
> possible in inotify.

I implemented it by always using constantly watching for all events
(IN_ALL_EVENTS) and storing the given user-flags with the callback etc.
When an event occurs, I check whether it matches the given mask.

For that to work, I had to restrict the flag-usage by the user to those
not having an effect on the shared descriptor.

I also added IN_EXCL_UNLINK as a default.  This avoids reporting events
for already deleted filenames, which are still opened by some process,
which seems what we want as a default.  

> Your changes look good; "make -C test filenotify-tests
> SELECTOR='$(SELECTOR_DEFAULT)'" passes all tests. Even if there is room
> for improvement I believe you could push your patch to master now, in
> order to get feedback from other developers.

I ran the non-expensive tests for inotify, kqueue and they succeeded.

I have no push privileges.