Re: PATCH: bug fix: infinite loop in sysread

[Roman Perepelitsa <roman.perepelitsa@gmail.com>]

On Wed, Feb 5, 2020 at 9:55 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> On Wed, Feb 5, 2020 at 6:20 AM Roman Perepelitsa
> <roman.perepelitsa@gmail.com> wrote:
> >
> > Here select() keeps returning 0, indicating timeout. This is not an
> > error, so errno doesn't get set. If it was EINTR prior to the call,
> > it stays EINTR, and the loop keeps spinning.
> >
> > The fix is to replace `< 1` with `< 0` in the loop condition.
>
> Wouldn't a better fix be to set errno = 0 before the call?

That would be incorrect. select() returns -1 on error and in general
functions must set errno on error and may set it on success. When
select() returns 0, errno can be anything.

Before sending this patch I've also checked if there are similar bugs
in other places. There are none. The result of select() is always
tested with `< 0` everywhere in Zsh expect this one place.

The function I'm modifying has a very similar branch that calls poll()
instead of select. That one compares with `< 0`.

Some quotes:

From `man select`:

       On success, select() and pselect() return the number of file
       descriptors  contained in the three returned descriptor sets
       (that is, the total number of bits that are set in  readfds,
       writefds,  exceptfds)  which  may  be  zero  if  the timeout
       expires before anything interesting happens.  On  error,  -1
       is  returned,  and  errno  is set to indicate the error; the
       file descriptor sets are  unmodified,  and  timeout  becomes
       undefined.

From `man errno`:

       The value in errno is significant only when the return value
       of the call indicated an error (i.e., -1  from  most  system
       calls;  -1  or NULL from most library functions); a function
       that succeeds is allowed to  change  errno.   The  value  of
       errno  is  never  set  to zero by any system call or library
       function.

Roman.