lkml.org 
[lkml]   [2010]   [Mar]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [GIT PULL] notification: after one week
From
Date
On Mon, 2010-03-08 at 21:57 +0000, Al Viro wrote:
> On Mon, Mar 08, 2010 at 03:55:39PM -0500, Eric Paris wrote:
> > Having you tell me to wait a week after rebasing (and probably being the
> > only person who waited a week after rebasing to ask for a pull) I'm
> > back. I hoped to hear some review but none came. If it does, rest
> > assured addressing those issues will be my top priority. Since the last
> > pull request the only change is that I corrected the build flags to not
> > add -Wsigned-pointer and I actually dropped the permissions bits from
> > this branch (those bits are and have been in linux-next for a long time
> > now as well though)
>
> Sigh... I *will* dig the full review out (been buried in autofs review
> lately), but for starters grep for fsnotify() and fsnotify_parent(),
> then tell me why on the earth are you doing that kind of insane multiplexors?
>
> I mean, WTF?
> ; git grep -n -w fsnotify_parent
> include/linux/fsnotify.h:28:static inline void fsnotify_parent(struct path *path, struct dentry
> include/linux/fsnotify.h:115: fsnotify_parent(NULL, dentry, mask);
> include/linux/fsnotify.h:176: fsnotify_parent(path, NULL, mask);
> include/linux/fsnotify.h:194: fsnotify_parent(path, NULL, mask);
> include/linux/fsnotify.h:212: fsnotify_parent(path, NULL, mask);
> include/linux/fsnotify.h:231: fsnotify_parent(path, NULL, mask);
> include/linux/fsnotify.h:247: fsnotify_parent(NULL, dentry, mask);
> include/linux/fsnotify.h:282: fsnotify_parent(NULL, dentry, mask);
> ;
>
> and *ALL* callers get one of those NULL and another non-NULL. With
> different behaviour inside that sucker. And fsnotify() is no better -
> it's a multiplexor from hell.

I have more out of tree work which makes fsnotify() (which does look
like it came straight from hell) a bit cleaner. I will clean both of
those interfaces up (mostly by duplicating the code of each into
multiple functions) and will include that in a later pull request.
Thanks for starting to look and I hope you don't find functional
failings.

Al also told me off list another thing he particularly hates style and
usage wise: FMODE_NONOTIFY and how it is overloaded with O_* in my
dentry_open() calls. It works, but we will think of a more manageable
solution (possibly completely separating FMODE_* and O_* at a higher
level. Again on the list of things to work on, but I don't believe you
indicated a show stopper today.....

-Eric

-Eric



\
 
 \ /
  Last update: 2010-03-08 23:47    [W:0.043 / U:0.628 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site