lkml.org 
[lkml]   [2004]   [Sep]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC][PATCH] inotify 0.10.0
    From
    Date
    On Mon, 2004-09-27 at 16:52 -0400, Robert Love wrote:
    > > > +struct inotify_event {
    > > > + int wd;
    > > > + int mask;
    > > > + int cookie;
    > > > + char filename[INOTIFY_FILENAME_MAX];
    > > > +};
    > >
    > > yeah, that's not very nice. Better to kmalloc the pathname.
    >
    > That is the structure that we communicate with to user-space.
    >
    > We could kmalloc() filename, but it makes the user-space use a bit more
    > complicated (and right now it is trivial and wonderfully simple).
    >
    > We've been debating the pros and cons.

    The current way pads out the structure unnecessarily, and still doesn't
    handle the really long filenames, by your admission. It incurs extra
    syscalls, as few filenames are really 256 characters in length. It makes
    userspace double-check whether the filename extends all the way to the
    boundary of the structure, and if so, then go back to the disk to try to
    guess what the kernel really meant to say.

    My way, doing a kmalloc of either the entire structure + filename length
    + NUL, or just the filename + NUL makes userspace infinitesimally
    trickier to write. There's more effort in just getting the error
    handling correct in either version.

    The current way viewed by userspace, stolen shamelessly from the beagle
    source:

    /* FIXME: We need to check for errors, etc. */
    remaining = sizeof (struct inotify_event);
    event_buffer = (char *) &event;
    while (remaining > 0) {
    num_bytes = read (fd, event_buffer, remaining);
    event_buffer += num_bytes;
    remaining -= num_bytes;
    }

    My way (where struct inotify_event is declared with a char filename[0]
    at the end):

    if ((n=read(fd, buf, sizeof buf)) > 0) {
    unsigned offset=0;
    while (n > 0) {
    // inotify guarantees complete events
    unsigned len = sizeof(struct inotify_event);
    dispatch((struct inotify_event *) &buf[offset]);
    len += strlen(buf[offset + len]) + 1;
    n -= len;
    offset += len;
    }
    }

    Doesn't seem all that tricky.

    I'm willing to believe that I'm missing something. Help me see what.

    Ray

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-03-22 14:06    [W:0.027 / U:0.188 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site