On Mon, 2012-12-17 at 00:33 +0100, Samuel Thibault wrote:
> Svante Signell, le Wed 12 Dec 2012 20:11:55 +0100, a écrit :
> > --- hurdselect_orig.c 2012-10-21 22:55:26.000000000 +0200
> > +++ hurdselect_step1.c 2012-12-12 19:29:26.000000000 +0100
> > @@ -37,6 +37,13 @@
> > each of READFDS, WRITEFDS, EXCEPTFDS that is nonnull. If TIMEOUT is not
> > NULL, time out after waiting the interval specified therein. Returns
> > the number of ready descriptors, or -1 for errors. */
> > +
> > +enum {
> > + DELAY = -1,
> > + SELECT = 0,
> > + POLL = 1
> > +} ispoll;
>
> Urgl, a global variable?! (and world-visible!!) That won't ever work
> with threads. Make it a variable local to the function.
OK, will move it into the function.
> > + /* FIXME: sigmask is only used for ppoll and pselect: not yet
> > implemented */
>
> What do you mean by not yet implemented?
ppoll and pselect are not implemented (yet), hence sigmask is NULL.
> > + /* FIXME: Only for ppoll */
> > if (sigmask)
> > __sigprocmask (SIG_SETMASK, &oset, NULL);
>
> Well, poll provides sigmask==NULL, so I don't think there's anything to
> fix here.
Since these functions are not implemented, why is the code there at all,
except as a placeholder? I added the comments as a reminder, but they
can easily be removed of course.
> > + /* FIXME: Only for pselect */
> > if (sigmask)
> > __sigprocmask (SIG_SETMASK, &oset, NULL);
>
> Ditto.
>
> > @@ -284,7 +614,7 @@ _hurd_select (int nfds,
> > /* Bogus answer; treat like an error, as a fake positive. */
> > type = SELECT_ALL;
> >
> > - /* This port is already ready already. */
> > + /* This port is already ready already. */
>
> Please avoid such spurious whitespace change.
Of course.
> I haven't (yet) read the rest of the details.
Richards patch "if (!err && got == 0 && firstfd != -1 && (firstfd !=
lastfd))" does not work, apt-get update (select-based) fails. So there
will be other means to avoid the double delay bug.
