Great, that sounds like the right fix. Just update patches 1 and 4 to do that and put it all in a branch and I'll pull it from there.
Kristian On Mar 13, 2011, at 4:26 PM, Iskren Chernev <[email protected]> wrote: > According to the epoll(7) man page, closing a file descriptor removes it from > all epoll sets as long as it hasn't been copied (and other copies are not > closed): > > Q6 Will closing a file descriptor cause it to be removed from all > epoll sets automatically? > > A6 Yes, but . . . (explanation about copied fs via dup* etc) > > So maybe just closing the fds is the cleanest solution, given that only > library code can manage them and won't copy them. > > Regards, > Iskren > > On Sun, Mar 13, 2011 at 9:33 PM, Iskren Chernev <[email protected]> > wrote: > Reply-To-All :) > > > ---------- Forwarded message ---------- > From: Iskren Chernev <[email protected]> > Date: 2011/3/13 > Subject: Re: minor fixes in event-loop.c > To: Kristian Høgsberg <[email protected]> > > > I fixed the "close fd" ones, but its not perfect: > -- in what order should we do the epoll_ctl, close and free? > -- what should we do if epoll_ctl is first and fails, or if close is first > and fails (call the next one or return)? > -- should we free if epoll_ctl of close fail? > -- should we loop on close while it returns EINTR - the man page doesn't say > weather SA_RESTART works for close. > > I think that close is unlikely to fail, because it is a special system fd. > I'm not sure how often does epoll_ctl fail and what can be done about it -- > there is no error returned from epoll_ctl that we might get unless there is > something very wrong. > > It should be good enough for now :) > > Regards, > Iskren > > PS.: I'll make a github account in the near future. So should I also send the > patches with send-mail or only somehow send links to the revision in github? > > 2011/3/13 Kristian Høgsberg <[email protected]> > On Sun, Mar 13, 2011 at 11:24 AM, Iskren Chernev > <[email protected]> wrote: > > Some patches for event-loop.c -- the signal & timer fds were never closed. > > In the future do you prefer patches standalone or archived? > > Looks like more good patches. Patches 1 and 4 close the fd before > passing it to epoll_ctl(), and I don't think that's how it's supposed > to work. It may or may not work depending on the implementation and > the man page doesn't state one way or the other. However, the obvious > safe approach is to just call epoll_ctl() before closing the fd. > > I prefer patches sent by git send-email, since they're easy to review > and discuss on the list, but for more than a couple of patches I also > would like a branch somewhere (github, gitorious etc) that I can just > pull if it all looks ok. > > Kristian > > >
_______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
