On Mon, 4 Mar 2024 16:03:55 GMT, Roger Riggs <rri...@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 > > src/java.base/share/classes/java/text/DecimalFormat.java line 3717: > >> 3715: // As maxFracDigits are fully displayed unlike maxIntDigits >> 3716: // Prevent OOME by setting to a much more reasonable value. >> 3717: setMaximumFractionDigits(DOUBLE_FRACTION_DIGITS); > > Setting a reasonable default makes sense. > In other control paths, the max fraction digits come from the inputs or are > explicitly set. > > It might be a reasonable related change to use StringBuilder.repeat() instead > of a loop at LIne 3312-3319, where the pattern char(s) are being appended to > the result. Thanks for taking a look. Updated the loop with `repeat` as you suggested, and decided to refactor the rest of the method while I was at it. Additionally, I added some more tests, as it seems that there is a lack of pattern tests for DecimalFormat in general. > In other control paths, the max fraction digits come from the inputs or are > explicitly set. Right, while `Integer.MAX_VALUE` can still be set by other control paths (and thus an OOME if toPattern() is invoked), this is something explicitly done by the user, and thus we decided we would still allow the behavior. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18094#discussion_r1511966791
