From: Alexander Barkov
Date: November 19 2010 4:22pm
Subject: Re: bzr commit into mysql-5.5-bugteam branch (bar:3135) Bug#58175
List-Archive: http://lists.mysql.com/commits/124493
Message-Id: <4CE6A445.8070105@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Hi Alik,

thanks for review!

Alexander Nozdrin wrote:
> Hi Alexander,
> 
> thank you for working on this.
> 
> As discussed on IRC, that function actually has another bug --
> the function returns (number_of_decimals + 1) instead
> of number_of_decimals.
> 
> The thing is that the function is not really used to return number
> of decimals (lol!), that's why this bug is not visible.
> 
> Although it seems pretty safe to fix also that new bug,
> it's proposed to leave it as it is because of 5.5 nature.
> 
> However, could you please
>   1) Report a new bug about that issue
>   2) Put a comment in the code about that flaw

I added a comment about this flaw:

+  /*
+    QQ:
+    The number of decimal digist in fact should be (str - decimal_point 
- 1).
+    But it seems the result of nr_of_decimals() is never used!
+
+    In case of 'e' and 'E' nr_of_decimals returns NOT_FIXED_DEC.
+    In case if there is no 'e' or 'E' parser code in sql_yacc.yy
+    never calls Item_float::Item_float() - it creates Item_decimal instead.
+
+    The only piece of code where we call Item_float::Item_float(str, len)
+    without having 'e' or 'E' is item_xmlfunc.cc, but this Item_float
+    never appears in metadata itself. Changing the code to return
+    (str - decimal_point - 1) does not make any changes in the test 
results.
+
+    This should be addressed somehow.
+    Looks like a reminder from before real DECIMAL times.
+  */


But I don't know if we really need to report it as a bug.


> 
> Wrt Bug#58175 itself, I think the patch is Ok and can be pushed.

Done. thanks!

> 
> Thanks!
> 
> On 18.11.2010 18:38, Alexander Barkov wrote:
>> #At file:///home/bar/mysql-bzr/mysql-5.5-bugteam.b58175/ based on 
>> revid:bar@stripped
>>
>>   3135 Alexander Barkov    2010-11-18
>>        Bug#58175 xml functions read initialized bytes when conversions 
>> happen
>>
>>        Problem:
>>
>>        nr_of_decimals could read behind the end of the buffer
>>        in case of a non-null-terminated string, which caused
>>        valgring warnings.
>>
>>        Fix:
>>
>>        fixing nr_of_decimals not to read behind the "end" pointer.
>>
>>      modified:
>>        mysql-test/r/xml.result
>>        mysql-test/t/xml.test
>>        sql/item.cc