On Thu, 12 Mar 2026 21:57:46 GMT, Alexey Ivanov <[email protected]> wrote:
>> Phil Race has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 8377568
>
> src/java.desktop/share/classes/java/awt/image/DataBuffer.java line 567:
>
>> 565: if ((i < 0) || ((offsets[bank] + i) < i)) {
>> 566: throw new ArrayIndexOutOfBoundsException("Index cannot be
>> negative : " + i);
>> 567: }
>
> This may also result in a confusing error message.
>
> Maybe, more index and offset into another helper method:
>
>
> private static void checkIndexOffset(int i, int offset) {
> if (i < 0) {
> throw new ArrayIndexOutOfBoundsException("Index cannot be
> negative : " + i);
> }
> if ((offset + i) < i) {
> throw new ArrayIndexOutOfBoundsException("(offset+i) cannot be
> negative : " +
> "(" + offset + " + " + i + ") = " + (offset + i));
> }
> }
>
>
> Then both checkIndex(int i) and checkIndex(int bank, int i) will call it like
> this
>
>
> checkIndexOffset(i, offset);
> checkIndexOffset(i, offsets[bank]);
>
>
> correspondingly?
I'll split out the check like the other case but I'm keeping the separate
methods so I can print the bank index in the 2nd case.
> src/java.desktop/share/classes/java/awt/image/DataBufferByte.java line 40:
>
>> 38: import java.util.Objects;
>> 39: import static sun.java2d.StateTrackable.State.STABLE;
>> 40: import static sun.java2d.StateTrackable.State.UNTRACKABLE;
>
> It's minor: I'd add a blank line between `Objects` and static imports.
>
> Usually, static imports go last and are separated from other imports by a
> blank line.
Not a pattern I'd ever noticed but I can do it for consistency
> src/java.desktop/share/classes/java/awt/image/DataBufferDouble.java line 29:
>
>> 27:
>> 28: import java.util.Objects;
>> 29: import static sun.java2d.StateTrackable.State.*;
>
> Suggestion:
>
> import java.util.Objects;
>
> import static sun.java2d.StateTrackable.State.STABLE;
> import static sun.java2d.StateTrackable.State.UNTRACKABLE;
>
> Add a blank line and expand the wildcards?
OK ... as above .. for consistency
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29766#discussion_r2935861578
PR Review Comment: https://git.openjdk.org/jdk/pull/29766#discussion_r2935853474
PR Review Comment: https://git.openjdk.org/jdk/pull/29766#discussion_r2935853957
