Re: Introducing a kqueue backend for GIO (GSoC)

[Dan Winship <danw gnome org>]

On 06/19/2011 04:25 PM, Dmitry Matveev wrote:
> My project includes writing a kqueue file monitoring backend for GIO, in
> order to provide fast file monitoring to Glib/GIO-powered applications
> on BSD systems.

Cool. I took a quick look. It mostly looks good. (Or at least, as good
as the inotify backend, which is pretty ugly in places :)

Issues:

    - The configure check is setting kqueue_support to true before
      checking that kqueue() and kevent() exist.

      I assume this is supposed to work on all BSDs? What about OS X?
      (If not, it probably needs additional checks to avoid building
      on platforms where it won't work.)

    - Re: kqueue-thread: Are you sure it's not possible add the kqueue
      fd to the main loop and just call kevent() whenever poll() says
      it's readable?

      (Longer term, there is a bug in bugzilla about moving the main
      loop to use epoll() instead of poll(), and if that ever happens
      it would probably make it pretty easy to switch the BSDs to using
      kqueue() instead of poll() too.)

    - "initialized" in _kh_startup() needs to be volatile or it's not
      guaranteed to behave the way you want...

    - There was a comment somewhere about using memory pools; in glib
      that would be gslice, and yes, it's good to use that if you're
      allocating lots of objects of the same size.

    - The write() call at the end of _kqueue_thread_func() needs to
      deal with the possibility of EINTR, I think. Or else you could
      just use some other communication mechanism, such as GAsyncQueue.
      (You might want to use that for the pick_up/removed fds lists
      too.)

    - Minor code style problems:

      Occasional missing spaces before the "(" in a function/macro call
      or around the "{" and "}" in a struct initialization.

      Some function arguments are not lined up right (the words should
      all line up, with the *s hanging off to the left, with an empty
      column between the longest type name and the left-most *). eg:

          void blah_blah_blah (int       *foo,
                               char     **bar,
                               gboolean   baz);

      A few ifs/fors/whiles are using non-gnu brace styles.

      Don't do "if (constant == variable)". Put the variable first. gcc
      will warn if you typed "=" instead of "==" anyway.

      If you're going to do function documentation in a "structured"
      way, use gtk-doc conventions.

      The use of a "g_" prefix on static/global variables seems very
      wrong to me (especially in files where nothing else is
      g-prefixed).

-- Dan