List:Commits« Previous MessageNext Message »
From:Sergey Vojtovich Date:December 8 2010 7:32pm
Subject:Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3521)
Bug#57450
View as plain text  
Hi Dmitry,

let me try to explain my point and please correct me if I'm
wrong.

When I saw this patch for the first time, I realized that
batch_readline_init() may be called with tty, socket, unnamed
pipe or whatever that has no name on filesystem. Calling fstat()
on something that has no associated file name looks suspicious,
but I don't know for sure if it is permitted or not.

That's why I asked:
  What if I run "mysql > abc", or in other words what if
  input_fd is a tty?
and later asked:
  What if I run my_fstat() on unnamed fd (e.g. tty, socket, unnamed
  pipe)?

But there is no answer whatsoever. Ok.

Later I realized that this is probably a regression caused by
fix for BUG#41486. As far as I can see fill_buffer() returns
-1 on error and 0 on eof. But with fix for BUG#41486
intern_read_line() ignores error from fill_buffer(), unless
buffer->eof is set.

So the main question is: what benefit gives hidding a bug in
intern_read_line() instead of restoring original logics?

Regards,
Sergey

On Wed, Dec 08, 2010 at 05:50:54PM +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 17:50:51 +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,13 @@ 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_IFDIR) == MY_S_IFDIR) ||
> +      ((input_file_stat.st_mode & MY_S_IFBLK) == MY_S_IFBLK))
> +    return 0;
> +
>    if (!(line_buff=(LINE_BUFFER*)
>          my_malloc(sizeof(*line_buff),MYF(MY_WME | MY_ZEROFILL))))
>      return 0;
> @@ -177,8 +185,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;
> -
> +  }
>    DBUG_PRINT("fill_buff", ("Got %lu bytes", (ulong) read_count));
>  
>    if (!read_count)
> @@ -222,6 +237,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;
> 


> 
> -- 
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe:    http://lists.mysql.com/commits?unsub=1


-- 
Sergey Vojtovich <svoj@stripped>
MySQL AB, Software Engineer
Izhevsk, Russia, www.mysql.com
Thread
bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3521) Bug#57450Dmitry Shulga8 Dec
  • Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3521)Bug#57450Sergey Vojtovich8 Dec
    • Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3521) Bug#57450Dmitry Shulga9 Dec
      • Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3521)Bug#57450Sergey Vojtovich9 Dec
        • Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3521) Bug#57450Dmitry Shulga13 Dec
          • Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3521)Bug#57450Sergey Vojtovich13 Dec