Hi Sergey!
On 07.12.2010, at 19:45, Sergey Vojtovich wrote:
> Hi Dmitry,
>
> a few questions inline...
>
>> 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?
Fixed.
>
>> + 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?
Fixed.
> Why at all to change this code? As far as I can see my_read() handles EINTR.
my_read handles EINTR if THREAD macro is ON. I don't know why did that, so I just added
unconditional handle of 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?
No. Moreover, I think that there is a bug in error handling in current implementation of
buffer filling. In current implementation
if call to my_read failed (when syscall read returned -1), then flag buffer->eof isn't
set to 1. Hence, code inside intern_read_line()
try make next attempt to fill_buffer (and consequently try to read from file descriptor),
although previous read failed and next attempt
don't make sense.
Regards, Dmitry.
