List:Commits« Previous MessageNext Message »
From:Davi Arnaut Date:January 10 2011 12:21pm
Subject:Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3521)
Bug#57450
View as plain text  
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
Thread
bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3521) Bug#57450Dmitry Shulga16 Dec
  • Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3521)Bug#57450Davi Arnaut10 Jan