Hi Dmitry,
a few questions inline...
On Fri, Dec 03, 2010 at 11:49:46AM +0000, Dmitry Shulga wrote:
> #At file:///Users/shulga/projects/mysql/5.1-bugteam-bug57450/ based on
> revid:azundris@stripped
>
> 3521 Dmitry Shulga 2010-12-03
> 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-03 11:49:35 +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,15 @@ 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;
> + int input_fd;
> +
> + input_fd= fileno(file);
> +
> + if (my_fstat(input_fd, &input_file_stat, MYF(0)) ||
> + (input_file_stat.st_mode & S_IFDIR))
Why not MY_S_ISDIR(input_file_stat.st_mode)?
Or at least not (input_file_stat.st_mode & MY_S_IFDIR)?
What if I run "mysql > abc", or in other words what if
input_fd is a tty?
> + return 0;
> +
> if (!(line_buff=(LINE_BUFFER*)
> my_malloc(sizeof(*line_buff),MYF(MY_WME | MY_ZEROFILL))))
> return 0;
> @@ -177,8 +187,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 (errno != EAGAIN && errno != EINTR)
Why not my_errno?
Why at all to change this code? As far as I can see my_read() handles EINTR.
> + buffer->eof= 1;
> return (size_t) -1;
> -
> + }
> DBUG_PRINT("fill_buff", ("Got %lu bytes", (ulong) read_count));
>
> if (!read_count)
> @@ -222,6 +239,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;
Don't you consider this way of returning "temporal error" too complex?
> }
> else
> continue;
>
Regards,
Sergey
--
Sergey Vojtovich <svoj@stripped>
MySQL AB, Software Engineer
Izhevsk, Russia, www.mysql.com
