Translate IN_ATTRIB to IN_MODIFY

[akapelrud]

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.

[GamePad64]

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.

[akapelrud]

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.

[GamePad64]

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.

[akapelrud]

@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.

[GamePad64]

Well, I have a patch with a compile-time switch: librevault@cddb575, 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.

[akapelrud]

@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.

[GamePad64]

Maybe, not a union, but a structure (or a bitset?) with extra information. I'll try to make a patch for it

[akapelrud]

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.

[berkus]

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?