List:Commits« Previous MessageNext Message »
From:Sergey Vojtovich Date:January 21 2011 9:01am
Subject:Re: bzr commit into mysql-5.1 branch (Dmitry.Shulga:3564) Bug#57450
View as plain text  
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=1

Regards,
Sergey
-- 
Sergey Vojtovich <svoj@stripped>
MySQL AB, Software Engineer
Izhevsk, Russia, www.mysql.com
Thread
bzr commit into mysql-5.1 branch (Dmitry.Shulga:3564) Bug#57450Dmitry Shulga20 Jan
  • Re: bzr commit into mysql-5.1 branch (Dmitry.Shulga:3564) Bug#57450Sergey Vojtovich21 Jan
    • Re: bzr commit into mysql-5.1 branch (Dmitry.Shulga:3564) Bug#57450Dmitry Shulga28 Jan
      • Re: bzr commit into mysql-5.1 branch (Dmitry.Shulga:3564) Bug#57450Sergey Vojtovich28 Jan