Re: Questions about patches for hurdselect.c

[Svante Signell]

On Fri, 2012-12-07 at 18:45 +0100, Svante Signell wrote:
> On Fri, 2012-12-07 at 17:35 +0100, Samuel Thibault wrote:
> > Svante Signell, le Fri 07 Dec 2012 17:13:00 +0100, a écrit :
> > > In the works of rewriting hurdselect.c I have some question on how to
> > > write patches. The changed code contains three parts:
> > > 1) Introducing three cases: DELAY, POLL, and SELECT
> > 
> > Why three cases? Isn't there a lot of common code between poll and
> > select?
> 
> Unfortunately the differences are too large to have them in the same
> code path, without loosing the overview. Additionally, the pure delay
> code is much faster than today's as reported in
> http://lists.gnu.org/archive/html/bug-hurd/2012-11/msg00000.html

As promised, attached is the first patch to split hurdselect.c code into
three cases: DELAY, POLL and SELECT, doing the first two parts as
promised in an earlier mail:
1) Introducing three cases: DELAY, POLL, and SELECT
2) Removing the unneeded code for each case statement

It does _not_ include any poll code updates yet, that will be in part 3,
i.e. the step 2 patch:
3) Rewriting the poll case to be compliant with POSIX 2001.

Attached is both a diff with the new code to the debian patched
hurdselect.c and the complete hurdselect_step1.c. 

Additional notes:

- The one FD patch by Richard is included, applied to both the POLL and
SELECT cases. Now the timeouts are correct (measured too).

- The pure delay case is much faster now, making it necessary to
introduce a delay of 1 msec when the timeout parameter is set to zero.
Otherwise the file descriptors will not be ready, and the test fails.
For test code see the link above.

- The flat delay of 1 msec is also needed in order to get ntpdate
working! The old implementation does also work now, but as mentioned
above, it does not with the new code. (and according to the debian
patch: vim)

- The union and some constant definitions have been moved to before the
three cases: DELAY, POLL and SELECT.

- Comments for the sigmask stuff, needed for ppoll and pselect has been
added. These functions are not yet implemented.

- Notes haver been added to where the code has a bug for the POLL case.
The updated POLL code will resolve mot of these bugs.

- Hopefully the code is more readable now, without causing regressions.

- Of course there might be indentation differences, not so clear
comments, etc, but this is the first version of the rewrite!

Comments welcomed, when hopefully accepted (after reviews) the POLL
update will follow. Further test code both for the DELAY, POLL and
SELECT cases is available if of interest.

Thanks,
Svante

hurdselect.c.patch
Description: Text Data

hurdselect_step1.c
Description: Text Data