minor fixes in event-loop.c

[Iskren Chernev]

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 <iskren.chernev at gmail.com>wrote:

> Reply-To-All :)
>
>
> ---------- Forwarded message ----------
> From: Iskren Chernev <iskren.chernev at gmail.com>
> Date: 2011/3/13
> Subject: Re: minor fixes in event-loop.c
> To: Kristian Høgsberg <krh at bitplanet.net>
>
>
> 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 <krh at bitplanet.net>
>
>> On Sun, Mar 13, 2011 at 11:24 AM, Iskren Chernev
>> <iskren.chernev at gmail.com> 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
>>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20110313/c253b324/attachment.html>