List:Commits« Previous MessageNext Message »
From:Luís Soares Date:June 29 2011 9:12am
Subject:Re: BUG#11747577
View as plain text  
OK. Testing it at the moment (while waiting for Nirbhay).

On 06/29/2011 10:12 AM, Sven Sandberg wrote:
> Hi again,
>
> I should also have said: the patch is ok to push, but do the small refactoring below
> if you like.
>
> /Sven
>
> On 06/29/2011 10:33 AM, Sven Sandberg wrote:
>>> === modified file 'client/mysql.cc'
>>> --- client/mysql.cc 2011-06-14 13:34:19 +0000
>>> +++ client/mysql.cc 2011-06-29 08:13:09 +0000
>>> @@ -1973,8 +1973,13 @@ static int read_and_execute(bool interac
>>> {
>>> if (!interactive)
>>> {
>>> + /*
>>> + batch_readline can return 0 on EOF or error.
>>> + In that case, we need to double check that we have a valid
>>> + line before actually setting line_length to read_length.
>>> + */
>>> line= batch_readline(status.line_buff, real_binary_mode);
>>> - line_length= status.line_buff->read_length;
>>> + line_length= line ? status.line_buff->read_length : 0;
>>> /*
>>> ASCII 0x00 is not allowed appearing in queries if it is not in binary
>>> mode.
>>> @@ -2004,7 +2009,11 @@ static int read_and_execute(bool interac
>>> (uchar) line[0] == 0xEF &&
>>> (uchar) line[1] == 0xBB &&
>>> (uchar) line[2] == 0xBF)
>>> + {
>>> line+= 3;
>>> + // decrease the line length accordingly to the 3 bytes chopped
>>> + line_length -=3;
>>> + }
>>> line_number++;
>>> if (!glob_buffer.length())
>>> status.query_start_line=line_number;
>>
>> This looks good. I would suggest one refactoring here: there are three
>> 'if' blocks in sequence that test for line!=NULL. To clarify the code,
>> can you move that check to an enclosing 'if'? I.e.:
>>
>> line= batch_readline
>> if (line)
>> {
>> line_length= status.line_buff->read_length;
>> if (!real_binary_mode && strlen(line) != line_length)
>> {...}
>> if (!line_number &&
>> (uchar) line[0] == 0xEF &&
>> (uchar) line[1] == 0xBB &&
>> (uchar) line[2] == 0xBF)
>> {...}
>> }
>>
>>
>> /Sven
>>
>

Thread
Re: BUG#11747577Sven Sandberg29 Jun
  • Re: BUG#11747577Sven Sandberg29 Jun
    • Re: BUG#11747577Luís Soares29 Jun