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:
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.
Ok, I'll go ahead and close 7088271 as a duplicate.
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?
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"
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
