mgorny added inline comments.
================
Comment at: clang/lib/DirectoryWatcher/DirectoryWatcher-linux.inc.h:135
+ if (!statusOpt.hasValue())
+ K = DirectoryWatcher::EventKind::Removed;
+ }
----------------
akyrtzi wrote:
> mgorny wrote:
> > akyrtzi wrote:
> > > mgorny wrote:
> > > > jkorous wrote:
> > > > > mgorny wrote:
> > > > > > Why? I suppose this deserves a comment.
> > > > > I'll add this comment:
> > > > >
> > > > > // The file might have been removed just after we received the event.
> > > > Wouldn't that cause removals to be reported twice?
> > > Not quite sure if it can happen in practice but I'd suggest to accept
> > > this as potential occurrence and add it to documentation ("a 'removed'
> > > event may be reported twice). I think it is better to handle a definite
> > > "fact" (the file doesn't exist) than ignore it and assume the 'removed'
> > > event will eventually show up, or try to eliminate the double reporting
> > > which would over-complicate the implementation.
> > >
> > > After all, if inotify() is not 100% reliable then there's already the
> > > possibility that you'll get a 'removed' event for a file that was not
> > > reported as 'added' before.
> > I see this as a partial workaround for race condition. You 'fix' it if
> > removal happens between inotify reporting the event and you returning it.
> > You don't if removal happens after you push it to events. Therefore, the
> > caller still needs to account for 'added' event being obsolete. I don't
> > really see a purpose in trying to workaround the problem partially here if
> > the caller still needs to account for it. Effectively, you're replacing one
> > normal case and one corner case with one normal case and two corner cases.
> I was mainly pointing out that the client already has to be prepared for a
> 'removed' event that does not have an associated 'added' event, regardless of
> what this code is doing. Therefore a potential double 'removed' event doesn't
> add complexity to the client.
>
> Could you clarify how the code should handle the inability to get the mod
> time ? Should it ignore the event ?
Given the code is supposed to wrap filesystem notification layer, I'd say it
should pass the events unmodified and not double-guess what the client expects.
The client needs to be prepared for non-atomicity of this anyway.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58418/new/
https://reviews.llvm.org/D58418
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
