Re: [RFC][PATCH] inotify 0.9.2

Ray Lee
2004-09-23T04:53:10Z

Okay, just skimmed the latest patch to try to make sure I'm not talking
crazy talk. No guarantees, though.

On Wed, 2004-09-22 at 20:42, John McCutchan wrote:
> the inotify kernel driver only allows userspace
> program to read in event sized chunks (So that the event queue
> handling is kept simple)

It's not much more code.

Instead of calculating events as:

+static ssize_t inotify_read(struct file *file, __user char *buf,
+ size_t count, loff_t *pos) {
[...]
+ int events;
[...]
+ /* We only hand out full inotify events */
+ if (count < sizeof(struct inotify_event)) {
+ out = -EINVAL;
+ goto out;
+ }
+
+ events = count / sizeof(struct inotify_event);

...just keep track of the actual byte count left in buf, and continue
copying until the next event would overflow buf. Require userspace to
provide a buffer at least NAME_MAX + sizeof(struct inotify_event) [*]
where the last field in the struct is declared as filename[0], which
will guarantee forward progress in passing events.

[*] Here's one of those things that makes me think that I'm
talking out my tush. The comments claim that only the filename
will be returned to userspace, but later on another comment says
that the size might technically fly up to PATH_MAX. Wassup?

Events still arrive at userspace in logical chunks; all is good.

Perhaps I'm missing something. Always a possibility, that.

BTW:
<pedantic>
+ unsigned long bitmask[MAX_INOTIFY_DEV_WATCHERS/BITS_PER_LONG];

would be more correct if written

unsigned long bitmask[(MAX_INOTIFY_DEV_WATCHERS + BITS_PER_LONG - 1) / BITS_PER_LONG];

</pedantic>

BTW #2: 'mask' is variously declared as an unsigned long and other times
as an int. Granted, the two base declarations seem to live in different
structs, but I can't figure out when a mask-like thing would want to be
signed. Please consider either changing the name or, more likely,
changing all usages to unsigned. My single linear reading through the
patch hasn't quite clarified the usage to me.

Ray

P.s. Have I mentioned that I like the inotify idea a heck of a lot
better than dnotify? Ghu save us from people who think signals are a
wonderful way to communicate complex information.