From: Sergey Vojtovich Date: January 21 2011 9:01am Subject: Re: bzr commit into mysql-5.1 branch (Dmitry.Shulga:3564) Bug#57450 List-Archive: http://lists.mysql.com/commits/129307 Message-Id: <20110121090113.GA13317@june> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Hi Dmitry, the patch looks nice - I like this idea of moving "truncated" flag to line buffer structure. But please find comments inline... On Thu, Jan 20, 2011 at 09:59:00AM -0000, Dmitry Shulga wrote: > #At file:///Users/shulga/projects/mysql/mysql-5.1-bug57450/ based on revid:martin.hansson@stripped > > 3564 Dmitry Shulga 2011-01-20 > 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 input source type. > > The solution is to stop reading data from standard input if a call > to read(2) failed. > > A new test case was added into mysql.test. > @ client/my_readline.h > Data members error and truncated was added to LINE_BUFFER structure. > These data members used instead of out parameters in functions > batch_readline, intern_read_line. > @ client/mysql.cc > read_and_execute() was modified: set status.exit_status to 1 > when a call to batch_readline() returns NULL and > the error data member of LINE_BUFFER structure is not equal to zero. > @ client/readline.cc > intern_read_line() was modified: cancel reading from input if > fill_buffer() returns -1, e.g. if call to read failed. > batch_readline was modified: set the error data member of LINE_BUFFER > structure to value of my_errno when system error happened during call > to my_read/my_realloc. > @ mysql-test/t/mysql.test > Test for bug#57450 was added. > > modified: > client/my_readline.h > client/mysql.cc > client/readline.cc > mysql-test/t/mysql.test > === modified file 'client/my_readline.h' > --- a/client/my_readline.h 2009-03-18 08:27:49 +0000 > +++ b/client/my_readline.h 2011-01-20 09:58:46 +0000 > @@ -25,9 +25,11 @@ typedef struct st_line_buffer > uint eof; > ulong max_size; > ulong read_length; /* Length of last read string */ > + int error; > + bool truncated; > } LINE_BUFFER; > > extern LINE_BUFFER *batch_readline_init(ulong max_size,FILE *file); > extern LINE_BUFFER *batch_readline_command(LINE_BUFFER *buffer, char * str); > -extern char *batch_readline(LINE_BUFFER *buffer, bool *truncated); > +extern char *batch_readline(LINE_BUFFER *buffer); > extern void batch_readline_end(LINE_BUFFER *buffer); > > === modified file 'client/mysql.cc' > --- a/client/mysql.cc 2010-11-26 13:57:59 +0000 > +++ b/client/mysql.cc 2011-01-20 09:58:46 +0000 > @@ -1872,14 +1872,13 @@ static int read_and_execute(bool interac > ulong line_number=0; > bool ml_comment= 0; > COMMANDS *com; > - bool truncated= 0; > status.exit_status=1; > > for (;;) > { > if (!interactive) > { > - line=batch_readline(status.line_buff, &truncated); > + line=batch_readline(status.line_buff); > /* > Skip UTF8 Byte Order Marker (BOM) 0xEFBBBF. > Editors like "notepad" put this marker in > @@ -1953,9 +1952,13 @@ static int read_and_execute(bool interac > if (opt_outfile && line) > fprintf(OUTFILE, "%s\n", line); > } > - if (!line) // End of file > + // End of file or system error > + if (!line) > { > - status.exit_status=0; > + if (status.line_buff->error) > + status.exit_status= 1; > + else > + status.exit_status= 0; > break; > } What if status.line_buff is NULL here? Besides, exit_status seem to be initialized to 1 by default. > > @@ -1976,7 +1979,8 @@ static int read_and_execute(bool interac > #endif > continue; > } > - if (add_line(glob_buffer,line,&in_string,&ml_comment, truncated)) > + if (add_line(glob_buffer,line,&in_string,&ml_comment, > + status.line_buff->truncated)) Coding style: add space after comma. > break; > } > /* if in batch mode, send last query even if it doesn't end with \g or go */ > > === modified file 'client/readline.cc' > --- a/client/readline.cc 2009-03-18 08:27:49 +0000 > +++ b/client/readline.cc 2011-01-20 09:58:46 +0000 > @@ -24,7 +24,7 @@ static bool init_line_buffer(LINE_BUFFER > ulong max_size); > static bool init_line_buffer_from_string(LINE_BUFFER *buffer,char * str); > static size_t fill_buffer(LINE_BUFFER *buffer); > -static char *intern_read_line(LINE_BUFFER *buffer, ulong *out_length, bool *truncated); > +static char *intern_read_line(LINE_BUFFER *buffer, ulong *out_length); > > > LINE_BUFFER *batch_readline_init(ulong max_size,FILE *file) > @@ -42,13 +42,12 @@ LINE_BUFFER *batch_readline_init(ulong m > } > > > -char *batch_readline(LINE_BUFFER *line_buff, bool *truncated) > +char *batch_readline(LINE_BUFFER *line_buff) > { > char *pos; > ulong out_length; > - DBUG_ASSERT(truncated != NULL); > > - if (!(pos=intern_read_line(line_buff,&out_length, truncated))) > + if (!(pos=intern_read_line(line_buff,&out_length))) Coding style: add space after comma. > return 0; > if (out_length && pos[out_length-1] == '\n') > if (--out_length && pos[out_length-1] == '\r') /* Remove '\n' */ > @@ -162,7 +161,10 @@ static size_t fill_buffer(LINE_BUFFER *b > if (!(buffer->buffer = (char*) my_realloc(buffer->buffer, > buffer->bufread+1, > MYF(MY_WME | MY_FAE)))) > - return (uint) -1; > + { > + buffer->error= my_errno; > + return (size_t) -1; > + } > buffer->start_of_line=buffer->buffer+start_offset; > buffer->end=buffer->buffer+bufbytes; > } > @@ -177,7 +179,10 @@ 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) > + { > + buffer->error= my_errno; > return (size_t) -1; > + } > > DBUG_PRINT("fill_buff", ("Got %lu bytes", (ulong) read_count)); > > @@ -199,7 +204,7 @@ static size_t fill_buffer(LINE_BUFFER *b > > > > -char *intern_read_line(LINE_BUFFER *buffer, ulong *out_length, bool *truncated) > +char *intern_read_line(LINE_BUFFER *buffer, ulong *out_length) > { > char *pos; > size_t length; > @@ -214,22 +219,25 @@ char *intern_read_line(LINE_BUFFER *buff > if (pos == buffer->end) > { > /* > - fill_buffer() can return 0 either on EOF in which case we abort > - or when the internal buffer has hit the size limit. In the latter case > - return what we have read so far and signal string truncation. > + fill_buffer() can return NULL on EOF (in which case we abort), > + on error, or when the internal buffer has hit the size limit. > + In the latter case return what we have read so far and signal > + string truncation. > */ > - if (!(length=fill_buffer(buffer)) || length == (uint) -1) > + if (!(length=fill_buffer(buffer))) Coding style: add space after equal sign. > { > if (buffer->eof) > DBUG_RETURN(0); > } > + else if (length == (size_t) -1) > + DBUG_RETURN(NULL); > else > continue; > pos--; /* break line here */ > - *truncated= 1; > + buffer->truncated= 1; > } > else > - *truncated= 0; > + buffer->truncated= 0; > buffer->end_of_line=pos+1; > *out_length=(ulong) (pos + 1 - buffer->eof - buffer->start_of_line); > DBUG_RETURN(buffer->start_of_line); > > === modified file 'mysql-test/t/mysql.test' > --- a/mysql-test/t/mysql.test 2010-12-01 06:55:31 +0000 > +++ b/mysql-test/t/mysql.test 2011-01-20 09:58:46 +0000 > @@ -555,5 +555,11 @@ DROP DATABASE connected_db; > --remove_file $MYSQLTEST_VARDIR/tmp/one_db_1.sql > --remove_file $MYSQLTEST_VARDIR/tmp/one_db_2.sql > > +# > +# Bug#57450: mysql client enter in an infinite loop if the standard input is a directory > +# > +--error 1 > +--exec $MYSQL < . > + > --echo > --echo End of tests > > > -- > MySQL Code Commits Mailing List > For list archives: http://lists.mysql.com/commits > To unsubscribe: http://lists.mysql.com/commits?unsub=svoj@stripped Regards, Sergey -- Sergey Vojtovich MySQL AB, Software Engineer Izhevsk, Russia, www.mysql.com