Code review request for #6469160, #7088271: (fmt) format("%.1g", 0.0) throws ArrayOutOfBoundsException

[Brandon Passanisi]

Hi David.  I have revised my fix for this CR.  Here's a description of 
the most recent changes:

    1. The code fix has been moved slightly from it's original position
    into a section within FormattedFloatingDecimal.java which handles
    0.0* numbers.

    2. The code fix no longer changes the precision value.

    3. The code fix checks to see if the precision is 1 and the number
    is a one-digit 0.  If not, the normal code which adds the decimal
    and remaining digits is run.  If so, this block of code is not run.

In addition, I have re-run the Basic-X set of Formatter tests to make 
sure there aren't any regressions.

    Webrev: http://cr.openjdk.java.net/~bpassani/6469160/1/webrev/
    <http://cr.openjdk.java.net/%7Ebpassani/6469160/1/webrev/>


Thanks.

On 1/24/2012 11:58 PM, David Holmes wrote:
> Hi Brandon,
>
> On 25/01/2012 5:03 PM, Brandon Passanisi wrote:
>> Hi David. Thank you for your review comments. My answers to your
>> questions are below:
>>
>> On 1/23/2012 9:56 PM, David Holmes wrote:
>>> More generally it is not clear to me that putting in this special case
>>> is the right way to fix this. Though I admit I don't really understand
>>> what the specification requires when you give a precision of 0 with a
>>> 'g' conversion:
>>>
>>> "If the conversion is 'g' or 'G', then the precision is the total
>>> number of digits in the resulting magnitude after rounding."
>>>
>>> So we asked for zero digits? What should that mean?
>>
>> The Formatter javadoc, within the "Float and Double" section, describes
>> the following regarding a value of 0 for precision and the 'g'/'G'
>> conversion [1]:
>>
>> "If the precision is 0, then it is taken to be 1"
>
> Ah! Thanks. I hadn't seen that (wish they wouldn't split up the spec 
> for this across different sections!).
>
> Okay that explains the 0/1 situation.
>
> But that makes me question the "rightness" of the fix even more. We 
> took steps to change a precision of 0 to 1, but then the fix changes 
> it back to 0 because otherwise something else breaks. Seems to me that 
> the "something else" that handles the precision of 1 incorrectly is 
> the code that really needs to be fixed. Further it suggest that there 
> may be assumptions in later code that precision is in fact never 0.
>
> David
> -----
>
>
>> The following code block within Formatter.java near line 3278 is run to
>> do this:
>>
>> if (precision == -1)
>> prec = 6;
>> else if (precision == 0)
>> prec = 1;
>>
>> And the precision value "prec" is given to FormattedFloatingDecimal.
>> Therefore, when this particular error condition is seen and the proposed
>> code fix is reached in FormattedFloatingDecimal.java, the precision will
>> be 1. So, the proposed code fix ends up supporting a precision value of
>> 0 and 1.
>>
>>
>> [1]: http://docs.oracle.com/javase/7/docs/api/java/util/Formatter.html
>>
>>
>> Thanks.
>>
>>>
>>> David
>>> -----
>>>
>>>> Since java has been throwing exceptions in these cases, I consulted 
>>>> with
>>>> the output of C's printf to make sure that the outputted strings 
>>>> are the
>>>> same. I updated the Formatter's Basic-X template of tests with a 
>>>> little
>>>> over 20 test format strings that were causing exceptions without the
>>>> change and the output of each is compared with the output from C's
>>>> printf with the same format string. And, I ran all of the Basic-X 
>>>> tests
>>>> to make sure there weren't any regressions in behavior.
>>>>
>>>> Thanks.
>>

-- 
Oracle <http://www.oracle.com>
Brandon Passanisi | Principle Member of Technical Staff


Green Oracle <http://www.oracle.com/commitment> Oracle is committed to 
developing practices and products that help protect the environment