Re: [PATCH] mi: Make sure fd is non-blocking before select+read.

[Jose E. Marchesi]

Hi Tim!

    @@ -166,12 +194,21 @@ static int
     pk_mi_loop (int fd)
     {
       fd_set active_fd_set, read_fd_set;
    +  int old_flags;
    +  int ret = 0;
    
       FD_ZERO (&active_fd_set);
       FD_SET (fd, &active_fd_set);
    
    +  /* Make sure that fd is non-blocking.
    +   * From 'man 2 select':
    +   *  On Linux, select() may report a socket file descriptor as
    +   *  "ready for reading", while nevertheless a subsequent read blocks." */
    +  old_flags = pk_mi_fd_set_nonblocking (fd);


The set_nonblocking/restore_blocking is OK, thanks for noticing this.

    +
       pk_mi_exit_p = 0;
    -  while (1)
    +
    +  while (ret != -1 && ret != -2 && pk_mi_exit_p == 0)

This change in the loop, I don't like.

Before, the logic involving `ret' was only in one place.

With the proposed change the logic is in two places, so if we add a new
`ret' value we may have to update _both_ the `while' condition and the
if..elseif at the bottom of the function.

Doesn't sound like an improvement to me...

         {
           read_fd_set = active_fd_set;
           if (select (FD_SETSIZE, &read_fd_set, NULL, NULL,
    @@ -182,20 +219,23 @@ pk_mi_loop (int fd)
             }
    
           if (FD_ISSET (fd, &read_fd_set))
    -        {
    -          int ret = pk_mi_read_from_client (fd);
    -
    -          if (pk_mi_exit_p)
    -            /* Client requested us to close the connection.  */
    -            return 1;
    -          else if (ret == -1)
    -            /* Client closed the connection.  */
    -            return 1;
    -          else if (ret == -2)
    -            /* Protocol error.  */
    -            return 0;
    -        }
    +        ret = pk_mi_read_from_client (fd);
         }
    +
    +  if (pk_mi_exit_p)
    +    /* Client requested us to close the connection.  */
    +    ret = 1;
    +  else if (ret == -1)
    +    /* Client closed the connection.  */
    +    ret = 1;
    +  else if (ret == -2)
    +    /* Protocol error.  */
    +    ret = 0;
    +
    +  /* Restore blocking mode if needed. */
    +  pk_mi_fd_restore_blocking (fd, old_flags);
    +
    +  return ret;
     }
    
     /* Message sender.  */
    --
    2.26.2