On Fri, 10 Apr 2026 18:36:28 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 546:
>
>> 544: final void checkBank(int bank) {
>> 545: if (bank < 0 || bank >= banks) {
>> 546: throw new ArrayIndexOutOfBoundsException("Bank index out of
>> range " + bank);
>
> Suggestion:
>
> throw new ArrayIndexOutOfBoundsException("Bank index out of range
> : " + bank);
>
> For consistency with other messages.
ok
> src/java.desktop/share/classes/java/awt/image/DataBuffer.java line 606:
>
>> 604: ((offset + size) <= 0) ||
>> 605: ((offset + size) > arrayLen) ||
>> 606: ((offset > 0) && ((offset + size ) < size));
>
> The last condition—`((offset > 0) && ((offset + size ) < size))`— can never
> be true because `((offset + size) <= 0)` would yield true if the sum
> overflows. By the time this third condition is reached, `offset >= 0` and
> `size > 0`, if the third condition yields false, then the last condition is
> bound to yield false too.
I like more explicit verification if it isn't too costly but OK.
> src/java.desktop/share/classes/java/awt/image/DataBufferByte.java line 122:
>
>> 120: * @throws NullPointerException if {@code dataArray} is {@code
>> null}.
>> 121: * @throws IllegalArgumentException if {@code size} is less than or
>> equal
>> 122: * to zero, or is greater than the length of {@code
>> dataArray}.
>
> The other subclasses except for `-Byte`, for example
> [`-Double`](https://github.com/openjdk/jdk/pull/29766/changes#diff-04543d1d9a796a3ba88b6fe9db5d132fac5c049d20e942c8431706811dbfa5cbR116)
> and
> [`-Float`](https://github.com/openjdk/jdk/pull/29766/changes#diff-8c62d99e15db8b04beec405481ad27a2c0d724eb5c4a0b9ffbaa49c97ed96d48R117),
> omit _“is”_:
>
> https://github.com/openjdk/jdk/blob/a4d19dde137d5d9019acdc30bcb863c28b77fc19/src/java.desktop/share/classes/java/awt/image/DataBufferDouble.java#L116-L117
>
> The other classes also use slightly different formatting.
>
> I prefer the version with “is”, it's clearer, as well as formatting here in
> the `-Byte` class.
This doesn't seem very important but OK.
> src/java.desktop/share/classes/java/awt/image/DataBufferByte.java line 152:
>
>> 150: * @throws IllegalArgumentException if {@code size} or {@code
>> offset}
>> 151: * is less than or equal to zero, or {@code (offset + size)}
>> 152: * is greater than the length of {@code dataArray}.
>
> This description is confusing because it implies `IllegalArgumentException`
> is thrown if
>
> * `size <= 0`, or
> * **`offset <= 0`**,
>
> but `offset == 0` is valid.
rewritten as
* @throws IllegalArgumentException
* if {@code size} is less than or equal to zero,
* or {@code offset} is less than zero,
* or {@code (offset + size)} is greater than the length of {@code
dataArray}.
> src/java.desktop/share/classes/java/awt/image/DataBufferDouble.java line 117:
>
>> 115: * @throws NullPointerException if {@code dataArray} is {@code
>> null}.
>> 116: * @throws IllegalArgumentException if {@code size} is less than or
>> equal to zero,
>> 117: * or greater than the length of {@code dataArray}.
>
> Suggestion:
>
> * @throws IllegalArgumentException if {@code size} is less than or equal
> * to zero, or is greater than the length of {@code dataArray}.
>
>
> To align with the version in `DataBufferByte.java`.
> https://github.com/openjdk/jdk/pull/29766/changes#r3066076500
ok
> src/java.desktop/share/classes/java/awt/image/DataBufferFloat.java line 375:
>
>> 373: * @return The data entry as a {@code float}.
>> 374: * @throws ArrayIndexOutOfBoundsException if {@code bank} is not a
>> valid bank index,
>> 375: * or {@code (i + getOffsets(bank)}} is not a valid index
>> into the bank.
>
> Suggestion:
>
> * @throws ArrayIndexOutOfBoundsException if {@code bank} is not a valid
> bank index,
> * or {@code (i + getOffsets()[bank])} is not a valid index into
> the bank.
fixed
> src/java.desktop/share/classes/java/awt/image/DataBufferShort.java line 216:
>
>> 214: * @throws NullPointerException if {@code offsets} is {@code null}.
>> 215: * @throws IllegalArgumentException if any element of {@code
>> offsets} is less than zero.
>> 216: * @throws IllegalArgumentException if the lengths of {@code
>> dataArray} and {@code offsets} differ.
>
> Suggestion:
>
> * @throws ArrayIndexOutOfBoundsException if the lengths of {@code
> dataArray} and {@code offsets} differ.
>
> The superclass throws `ArrayIndexOutOfBoundsException` in this case.
fixed
> src/java.desktop/share/classes/java/awt/image/DataBufferUShort.java line 83:
>
>> 81: if (size <= 0) {
>> 82: throw new IllegalArgumentException("Size must be > 0");
>> 83: }
>
> The `if` condition is redundant.
obsolete. removed
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29766#discussion_r3068934641
PR Review Comment: https://git.openjdk.org/jdk/pull/29766#discussion_r3068971077
PR Review Comment: https://git.openjdk.org/jdk/pull/29766#discussion_r3068948006
PR Review Comment: https://git.openjdk.org/jdk/pull/29766#discussion_r3068954973
PR Review Comment: https://git.openjdk.org/jdk/pull/29766#discussion_r3068956389
PR Review Comment: https://git.openjdk.org/jdk/pull/29766#discussion_r3068962044
PR Review Comment: https://git.openjdk.org/jdk/pull/29766#discussion_r3068963079
PR Review Comment: https://git.openjdk.org/jdk/pull/29766#discussion_r3068964008
