Code review request for #6469160, #7088271 (original) (raw)

David Holmes david.holmes at oracle.com
Tue Jan 24 05:56:41 UTC 2012

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/64691607088271/1/ <http://cr.openjdk.java.net/%7Ebpassani/64691607088271/1/> Bug #6469160: http://bugs.sun.com/bugdatabase/viewbug.do?bugid=6469160 Bug #7088271: http://bugs.sun.com/bugdatabase/viewbug.do?bugid=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:

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.

More information about the core-libs-dev mailing list