Re: [PATCH,HURD] hurdselect: Step1, code split preparations

[Svante Signell]

On Tue, 2013-01-22 at 19:15 +0100, Pino Toscano wrote:
> Hi,
> 
> Alle martedì 22 gennaio 2013, Svante Signell ha scritto:
> > Attached is the first patch for a 3-way split of hurdselect.c into
> > three cases: DELAY, POLL, SELECT
> 
> What's the use of of the separate DELAY case?
> Basically, it seems to be optimizing just a very specific case, i.e.:
>   select(0, ..., &timeout);
> while, on the other hand, code like
>   fd_set empty;
>   FD_ZERO(&empty);
>   select(1, &empty, &empty, &empty, &timeout);
> (or with any nfd > 0) with your changes would be considered as SELECT.

If nfds>0 it depends on pollfds if the case is POLL or SELECT?
 
>From the man page for select:
Some code calls select() with all three sets empty, nfds zero, and a
non-NULL timeout as a fairly portable way to sleep with subsecond
precision.

And of course calling poll with nfds=0 is equal to a delay too if
timeout != 0

> After all, the code checking the arguments should take care of handling
> the "no descriptors" situations already, so the separate DELAY is
> simply redundant (being a subset of SELECT).
> 
> > leading to a more POSIX conforming POLL.
> 
> theorically, poll could be fixed also without restructing hurdselect...

We have discussed this issue before. Let's see what others say.

> > Starting point is the hurdselect.c created by all Debian patches
> > applied up to eglibc-2.13-38 + 3 additional patches:
> 
> Note that if you want to send patches upstream, your code must apply
> on master branch of glibc.git, not on some old glibc version with
> distribution-specific patches.

OK, will resend to debian-hurd instead...

> > +      union
> > +       {
> > [...]
> 
> This block must be properly indented.

The code was moved, therefore the missing indentation update. Is this
really important at this stage?