Hi Dmitry,
On 12/16/10 3:17 PM, Dmitry Shulga wrote:
> #At file:///Users/shulga/projects/mysql/5.1-bugteam-bug57450/ based on
> revid:azundris@stripped
>
> 3521 Dmitry Shulga 2010-12-16
> 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
monitor -> client.
> 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.
>
> Additionally, it was fixed a bug in processing of error returned by
> read syscal.
syscal -> syscall. Or just use the more common notation read(2).
Patch is not approved, comments below.
> @ client/readline.cc
> batch_readline_init() was modified: added checking for file mode
> and returning from function if input stream is a directory.
>
> intern_read_line() was modified: cancel reading from input if
> fill_buffer() returns -1, e.g. if call to read failed.
Please also add a test case. Something like:
=== modified file 'mysql-test/t/mysql.test'
--- mysql-test/t/mysql.test 2010-12-01 06:58:21 +0000
+++ mysql-test/t/mysql.test 2011-01-10 12:08:28 +0000
@@ -568,5 +568,13 @@ DROP DATABASE connected_db;
--remove_file $MYSQLTEST_VARDIR/tmp/one_db_1.sql
--remove_file $MYSQLTEST_VARDIR/tmp/one_db_2.sql
---echo
+--echo #
+--echo # Bug#57450 mysql client enter in an infinite loop if the
standard input is a directory
+--echo #
+
+--mkdir $MYSQLTEST_VARDIR/bug57450
+--error 1
+--exec $MYSQL < $MYSQLTEST_VARDIR/bug57450
+--rmdir $MYSQLTEST_VARDIR/bug57450
+
--echo End of tests
> modified:
> client/mysql.cc
> 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-16 17:16:56 +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(MY_WME)) ||
> + MY_S_ISDIR(input_file_stat.st_mode) ||
> + MY_S_ISBLK(input_file_stat.st_mode))
> + return 0;
If the file is not a directory or block device and yet read() fails with
a non-transient error, we will still have the infinite loop problem.
I also tend to agree with Sergey that this might be problematic
portability-wise and does not really cover all possible cases.
I liked better the approach taken in one of the earlier patches where a
failure to read(2) was handled properly.
Regards,
Davi
