Fwd: minor fixes in event-loop.c

[Iskren Chernev]

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/436bfc63/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Close-timer-file-descriptors-in-event-loop-on-remove.patch
Type: text/x-patch
Size: 1408 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20110313/436bfc63/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-Close-signal-file-descriptor-in-event-loop-on-remove.patch
Type: text/x-patch
Size: 1425 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20110313/436bfc63/attachment-0001.bin>