Code review request for #6469160, #7088271

[David Holmes]

Hi Brandon,

On 21/01/2012 4:19 AM, Brandon Passanisi wrote:
> Resending again...
>
> Hello core-libs. I was wondering of somebody could be please review the
> following fix for #6469160 and #7088271. The changes in the webrev fix
> both bugs. Information is below:
>
> Webrev URL: http://cr.openjdk.java.net/~bpassani/6469160_7088271/1/
> <http://cr.openjdk.java.net/%7Ebpassani/6469160_7088271/1/>
> Bug #6469160: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6469160
> Bug #7088271: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7088271

Seems to me that 7088271 should be closed as a duplicate of 6469160.

> Both bugs uncover the current behavior where using a 0 or 1 precision
> value with a float zero causes an ArrayIndexOutOfBoundsException. The
> current code in FormattedFloatingDecimal.java interprets the float zero
> as "0.0" in the case where precision is 0 or 1 and returns the length of
> it's characters as 3. Later in Formatter.addZeros(), the character array
> "0.0" is passed in, but a new array of only 1 character is allocated.
> When an System.arraycopy() is performed, the
> ArrayIndexOutOfBoundsException occurs. In fact, when run with "-esa" an
> AssertionError occurs at "assert (outPrec <= prec);" on line 3393 of
> Formatter.java. The fix is for FormatedFloatingDecimal.java to interpret
> the float zero as a single "0" because of the precision being set to 0
> or 1.

This isn't an area I'm familiar with so please excuse me if I'm missing 
something obvious. I'm confused about the fix in regards to the two 
cases reported in the CRs. In one case we have:

String.format("%3.0g", 0.0);

where the precision is 0. But in your fix you only special case the 
situation where precision is 1:

+           if (digits.length == 1 && digits[0] == '0'
+               && precision == 1) {
+               // When the number is zero and precision is 1, set the
+               // precision to 0 so that a decimal point and digits
+               // are not added by code later in this method.
+               precision--;
+            } else {

so I don't understand how this fixes %3.0g ?

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?

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.