On Tue, 10 Dec 2024 12:24:34 GMT, Claes Redestad <redes...@openjdk.org> wrote:
>> Shaojin Wen 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 17 additional >> commits since the last revision: >> >> - Merge remote-tracking branch 'upstream/master' into >> int_get_chars_dedup_202411 >> - Merge remote-tracking branch 'upstream/master' into >> int_get_chars_dedup_202411 >> - Merge remote-tracking branch 'upstream/master' into >> int_get_chars_dedup_202411 >> - fix unsafe address overflow >> - add benchmark >> - remove comments, from @liach >> - Merge remote-tracking branch 'upstream/master' into >> int_get_chars_dedup_202410 >> - fix Helper >> - fix Helper >> - fix Helper >> - ... and 7 more: https://git.openjdk.org/jdk/compare/f354c08b...a05c2f5f > > src/java.base/share/classes/java/math/BigDecimal.java line 4213: > >> 4211: if (intCompact != INFLATED) { >> 4212: coeff = new char[19]; >> 4213: offset = DecimalDigits.getChars(Math.abs(intCompact), >> coeff.length, coeff); > > `StringBuilderHelper.putIntCompact` asserts that `Math.abs(intCompact)` is > non-negative, but there is no such assert in `DecimalDigits.getChars`. I'm > not too familiar with `BigDecimal` and maybe `intCompact` wouldn't be set for > `Long.MIN_VALUE`. If so perhaps this should use `StrictMath#absExact(long)` > for clarity rather than lean on an assert? static final long INFLATED = Long.MIN_VALUE; INFLATED is Long.MIN_VALUE, and a pre-judgment is made here if (intCompact != INFLATED) { offset = DecimalDigits.getChars(Math.abs(intCompact), coeff.length, coeff); } So Math.abs(intCompact) here will not have overflow problems ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22023#discussion_r1878234662
