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

[Pino Toscano]

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.

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...

> 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.

> +      union
> +       {
> [...]

This block must be properly indented.

>             const int fd = (int) d[i].io_port;
>  
> -           if (fd < _hurd_dtablesize)
> -             {
>                 d[i].cell = _hurd_dtable[fd];
>                 d[i].io_port = _hurd_port_get (&d[i].cell->port,
> &d[i].ulink); if (d[i].io_port != MACH_PORT_NULL)
>                   continue;
> -             }

why is this check removed? When collecting inputs from poll,
d[i].io_port is the fd passed, which has no guarantee to be lower than
_hurd_dtablesize.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.