Re: [RFC][PATCH] inotify 0.9.2

Ray Lee
2004-09-23T05:29:21Z

I still might be talking crazy talk, but...

On Wed, 2004-09-22 at 22:10, Robert Love wrote:
> The comment is just a warning, though, to explain the dreary theoretical
> side of the world. Pragmatism demands that we just use
> INOTIFY_FILENAME_MAX, which is a more reasonable 256.

Why bother being pragmatic when we can be correct? It's not much more
code to just do it right, and allow up to PATH_MAX filenames to be
passed to userspace.

As an alternate argument, an `ls -1 | wc` on a randomly picked directory
of my filesystem reveals an average filename length of just under 11
characters. We can save some memory (and a lot of syscalls!) by packing
in events more tightly than the 256 character statically sized
rendition.

So, correct *and* efficient. Again, what am I missing here?

> > 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>
>
> Indeed! Although we define MAX_INOTIFY_DEV_WATCHERS right above and it
> is a power of two.

Sure, it's a multiple of BITS_PER_LONG right *now*, but what about in
the future? It only adds compile time overhead. Regardless, it's why I
wrote my comment as 'more correct.' I won't lose any sleep over that
change not going in, but there's no reason to encourage bad coding
habits.

> Oh, dude, inotify is a godsend.

Amen. So let's get it nailed, so that this corner of the problem space
can be considered done, and we can all move on to building bigger and
better things on top of it.

Ray