From: Dmitry Shulga
Date: January 28 2011 2:17pm
Subject: Re: bzr commit into mysql-5.1 branch (Dmitry.Shulga:3564) Bug#57450
List-Archive: http://lists.mysql.com/commits/129881
Message-Id: <4A8BB8E3-4B44-4DF0-8A40-04D4FCD164C7@oracle.com>
MIME-Version: 1.0 (Apple Message framework v1082)
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: quoted-printable

Hi Sergey.

> Hi Dmitry,
>=20
> the patch looks nice - I like this idea of moving "truncated" flag
> to line buffer structure. But please find comments inline...
>=20
>> +    if (!line)
>>     {
>> -      status.exit_status=3D0;
>> +      if (status.line_buff->error)
>> +        status.exit_status=3D 1;
>> +      else
>> +        status.exit_status=3D 0;
>>       break;
>>     }
> What if status.line_buff is NULL here?
I don't see how can a status.line_buff have a NULL value in this place. =
If you have a test case where status.line_buff have a NULL value=20
please add new bug report.

=20
>=20
> Besides, exit_status seem to be initialized to 1 by default.
Yes, by default exit_status is set to 1.=20
Nevertheless, I think that explicit assignment of status.exit_status to =
1 or 0 make this code snippet clearer

>>=20
>> @@ -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.
Done

>> -char *batch_readline(LINE_BUFFER *line_buff, bool *truncated)
>> +char *batch_readline(LINE_BUFFER *line_buff)
>> {
>>   char *pos;
>>   ulong out_length;
>> -  DBUG_ASSERT(truncated !=3D NULL);
>>=20
>> -  if (!(pos=3Dintern_read_line(line_buff,&out_length, truncated)))
>> +  if (!(pos=3Dintern_read_line(line_buff,&out_length)))
> Coding style: add space after comma.
Done

>> -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 =3D=3D 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=3Dfill_buffer(buffer)) || length =3D=3D (uint) =
-1)
>> +      if (!(length=3Dfill_buffer(buffer)))
> Coding style: add space after equal sign.
Done