From: Martin Hansson
Date: December 28 2010 8:33am
Subject: Re: bzr commit into mysql-5.1-bugteam branch (martin.hansson:3534)
 Bug#58165
List-Archive: http://lists.mysql.com/commits/127618
Message-Id: <4D19A0D3.7020400@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Hi Sergey,

Sergey Glukhov skrev 2010-12-23 12.44:
> Hi Martin, see comments below
>
> On 12/22/2010 03:30 PM, Martin Hansson wrote:
>> #At file:///data0/martin/bzrroot/bug58165/5.1bt-lazy_copy/ based on 
>> revid:sergey.glukhov@stripped
>>
>>
>>      modified:
>>        mysql-test/r/func_str.result
>>        mysql-test/t/func_str.test
>>        sql/item_strfunc.cc
>>        sql/sql_string.cc
> ...
>> === modified file 'sql/item_strfunc.cc'
>> --- a/sql/item_strfunc.cc    2010-11-11 10:25:23 +0000
>> +++ b/sql/item_strfunc.cc    2010-12-22 12:30:13 +0000
>> @@ -1305,8 +1305,15 @@ String *Item_func_substr_index::val_str(
>>     null_value=0;
>>     uint delimiter_length= delimiter->length();
>>     if (!res->length() || !delimiter_length || !count)
>> -    return&my_empty_string;        // Wrong parameters
>> -
>> +  {
>> +    if (str->copy("", 0, default_charset_info))        // Wrong 
>> parameters
>> +    {
>> +      /* Out of memory */
>> +      null_value= true;
>> +      return NULL;
>> +    }
>> +    return str;
>> +  }
>>     res->set_charset(collation.collation);
>>
>>   #ifdef USE_MB
>>
>
> Using my_empty_string is not safe, the bug shows it.
> So we should remove my_empty_string from all places
> where it is used. 
Removing my_empty_string altogether is way too large to be done as an 
MRU bug fix (Believe me, I tried.) I suggest filing a separate report 
about it. Let's make this customer happy first and then we'll deal with 
the other problems we discovered while working on it.
> I discussed the problem with Bar,
> he suggested the following fix:
>
> === modified file 'sql/item_strfunc.cc'
> --- old/sql/item_strfunc.cc    2010-11-11 10:25:23 +0000
> +++ new/sql/item_strfunc.cc    2010-12-23 11:29:58 +0000
> @@ -1305,7 +1305,10 @@
>    null_value=0;
>    uint delimiter_length= delimiter->length();
>    if (!res->length() || !delimiter_length || !count)
> -    return &my_empty_string;        // Wrong parameters
> +  {
> +    str_value.set("", 0, collation.collation);  // Also safe in case 
> of OOM error as no allocations happen
> +    return &str_value;
> +  }
Much better solution, I admit. Done.
>
> PS I assigned Bar as second reviewer, hope you don't mind...
Not at all, probably a wise move in fact. :-)

/Martin