berkus
Translate IN_ATTRIB to IN_MODIFY
Inotify has a separate event for attribute changes (changes to permissions, timestamps, etc.) (see inotify man pages). Translate IN_ATTRIB to a dir_monitor_event::modified event, to be consistent with e.g. the windows implementation.
Oh, inotify is a difficult topic. For example, I think, it would be good to change IN_MODIFY with IN_CLOSE_WRITE to be consistent with Windows.
What if some application writing to a file decides to keep the file descriptor of said file lingering after writing (i.e. by not closing it)? Then no event would be reported by dir_monitor if we are only listening to IN_CLOSE_WRITE.
I guess the problem with inotify is that over-reports write events, but in my book over-reporting is better than under-reporting. I think it should be up to the user of dir_monitor to combine multiple events into one, not the library.
By not listening for IN_ATTRIB dir_monitor disregards important file change events.
I fully agree with you about IN_ATTRIB.
But I think that this should be up to user to select between IN_CLOSE_WRITE and IN_MODIFY.
@GamePad64 Good that we agree. Yes, having an option for choosing is a good idea, but how would you implement it? -remember that inotify is only one out of 4 (current) implementations, so adding this to the main interface is probably a bad idea. A compiletime switch (macro or policy through templating) could work though, as dir_monitor is a header-only library.
Well, I have a patch with a compile-time switch: https://github.com/Librevault/dir_monitor/commit/cddb57519efa38cc51b7f67ea7cf3ffe3237db18, but I think it is an ugly solution, because this switch is library-wide, and I cannot create two dir_monitors, one with IN_MODIFY, and another with IN_CLOSE_WRITE.
@GamePad64 Maybe listening for both IN_CLOSE_WRITE and IN_MODIFY and translate both to dir_monitor_event::modified, then add a union member to dir_monitor_event to enable passing of extra information for each event type. In this case a "close" flag. Likewise, if the source event is IN_ATTRIB we could add something like the modification time (last write time) or whatever else is necessary.
Maybe, not a union, but a structure (or a bitset?) with extra information. I'll try to make a patch for it
By putting your structure into a union you can have different structures based on the event type occupy the same memory space. This makes it easier to add extra flags/fields to other event types later on. Have a look at how this was done in the Simple DirectMedia Layer (SDL) event union, or even in Xlib.
Sorry guys, I was busy for a while but will get back and review the PRs.
Is there anything changed here or the code seems good to go to you?
