From: Sergey Vojtovich
Date: December 8 2010 2:36pm
Subject: Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3521)
 Bug#57450
List-Archive: http://lists.mysql.com/commits/126328
Message-Id: <20101208143651.GA23349@june>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii

Hi Dmitry,

On Wed, Dec 08, 2010 at 09:42:59AM +0000, Dmitry Shulga wrote:
> #At file:///Users/shulga/projects/mysql/5.1-bugteam-bug57450/ based on revid:azundris@stripped
> 
>  3521 Dmitry Shulga	2010-12-08
>       Fixed bug#57450	- mysql client enter in an infinite loop
>       if the standard input is a directory.
>       
>       The problem is that mysql monitor try to read from stdin without
>       checking for type of input source.
>       
>       The solution is to check if stdin's file descriptor related to directory
>       and exit from program if this case is true.
>      @ client/readline.cc
>         batch_readline_init() was modified: added checking for file mode
>         and returning from function if input stream is a directory.
>         
>         fill_buffer() and intern_read_line() was modified: doesn't set eof flag
>         if read failed with temporal error and try repeat this failed syscall.
> 
>     modified:
>       client/readline.cc
> === modified file 'client/readline.cc'
> --- a/client/readline.cc	2009-03-18 08:27:49 +0000
> +++ b/client/readline.cc	2010-12-08 09:42:55 +0000
> @@ -18,6 +18,7 @@
>  #include <my_global.h>
>  #include <my_sys.h>
>  #include <m_string.h>
> +#include <my_dir.h>
>  #include "my_readline.h"
>  
>  static bool init_line_buffer(LINE_BUFFER *buffer,File file,ulong size,
> @@ -30,6 +31,12 @@ static char *intern_read_line(LINE_BUFFE
>  LINE_BUFFER *batch_readline_init(ulong max_size,FILE *file)
>  {
>    LINE_BUFFER *line_buff;
> +  MY_STAT input_file_stat;
> +
> +  if (my_fstat(fileno(file), &input_file_stat, MYF(0)) ||
> +      !(input_file_stat.st_mode & (MY_S_IFREG|MY_S_IFIFO)))
> +    return 0;
> +
I asked: "What if I run "mysql > abc", or in other words what if
input_fd is a tty?"
You answered: "Fixed."
But my question still applies. Let me try to ask it differently.
What if I run my_fstat() on unnamed fd (e.g. tty, socket, unnamed
pipe)?

>    if (!(line_buff=(LINE_BUFFER*)
>          my_malloc(sizeof(*line_buff),MYF(MY_WME | MY_ZEROFILL))))
>      return 0;
> @@ -177,8 +184,15 @@ static size_t fill_buffer(LINE_BUFFER *b
>    /* Read in new stuff. */
>    if ((read_count= my_read(buffer->file, (uchar*) buffer->end, read_count,
>  			   MYF(MY_WME))) == MY_FILE_ERROR)
> +  {
> +    /*
> +      we don't set eof flag for this temporal errors because next
> +      call to read may be successful.
> +    */
> +    if (my_errno != EAGAIN && my_errno != EINTR)
> +      buffer->eof= 1;
>      return (size_t) -1;
Thanks for your explanations. Now I better understand what you're
trying to achieve here.

I'd suggest to study:
http://lists.mysql.com/commits/69536

If you see no problems with that patch, let me study it once again.

As far as I can see the problem you're trying to hide here is a
regression caused by that bugfix.

Regards,
Sergey

> -
> +  }
>    DBUG_PRINT("fill_buff", ("Got %lu bytes", (ulong) read_count));
>  
>    if (!read_count)
> @@ -222,6 +236,9 @@ char *intern_read_line(LINE_BUFFER *buff
>        {
>          if (buffer->eof)
>            DBUG_RETURN(0);
> +        // check if syscall read failed with temporal error.
> +        else if (length == (uint) -1)
> +          continue;
>        }
>        else
>          continue;
> 

-- 
Sergey Vojtovich <svoj@stripped>
MySQL AB, Software Engineer
Izhevsk, Russia, www.mysql.com