RFR: JDK-8326908: DecimalFormat::toPattern throws OutOfMemoryError when pattern is empty string [v2]
Justin Lu
jlu at openjdk.org
Tue Mar 5 21:27:11 UTC 2024
On Tue, 5 Mar 2024 19:10:19 GMT, Naoto Sato <naoto at openjdk.org> wrote:
>> Justin Lu has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
>>
>> - cleanup/typo
>> - Merge branch 'master' into JDK-8326908-emptyPattern-OOME
>> - implement feedback + improve pattern related tests
>> - minor additions
>> - init
>
> test/jdk/java/text/Format/DecimalFormat/ToPatternTest.java line 60:
>
>> 58: dFmt.setMinimumFractionDigits(minFrac);
>> 59:
>> 60: String[] patterns = dFmt.toPattern().split("\\.");
>
> Instead of assuming/hardcoding the decimal separator, `DecimalFormatSymbols.getInstance(Locale.US).getDecimalSeparator()` may be better.
Actually, since `toPattern()` is non-localized, it will always use "." as specified by DecimalFormat.
I removed the misleading comment `// Use a US dFmt, which uses '.' as the decimal separator` that indicated we needed to use a DecimalFormatSymbols where "." was the decimal separator. Can just simply use a DecimalFormat returned by the default constructor.
> test/jdk/java/text/Format/DecimalFormat/ToPatternTest.java line 110:
>
>> 108: // 8326908: Verify that an empty pattern DecimalFormat does not throw an
>> 109: // OutOfMemoryError when toPattern() is invoked. Behavioral change of
>> 110: // MAXIMUM_INTEGER_DIGITS replaced with DOUBLE_FRACTION_DIGITS for empty
>
> Should we explicitly check `new DecimalFormat("").getMaximumFractionDigits() == DOUBLE_FRACTION_DIGITS`?
Thanks for the review, added an explicit check
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18094#discussion_r1513517348
PR Review Comment: https://git.openjdk.org/jdk/pull/18094#discussion_r1513517336
More information about the core-libs-dev
mailing list