Re: Questions about patches for hurdselect.c

[Svante Signell]

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.