On Fri, 6 Mar 2026 18:19:22 GMT, Phil Race <[email protected]> wrote:
>> This fix updates DataBuffer subclasses to actually adhere to their stated
>> specifications by rejecting certain invalid parameters for constructors and
>> getters and setters.
>> A new egression test for each of the constructor and getter/setter cases is
>> supplied.
>>
>> No existing regression tests fail with this change, and standard demos work.
>>
>> Problems caused by these changes are most likely to occur if the client has
>> a bug such that
>> - a client uses the constructors that accept an array and then supplies a
>> "size" that is greater than the array.
>> - a client uses the constructors that accept an array and then supplies a
>> "size" that is less than the array and then uses getter/setters that are
>> within the array but outside the range specified by size.
>>
>> Since very few clients (and just one case in the JDK that I found) even use
>> these array constructors the changes are unlikely to make a difference to
>> clients.
>>
>> The CSR is ready for review https://bugs.openjdk.org/browse/JDK-8378116
>
> Phil Race has updated the pull request incrementally with one additional
> commit since the last revision:
>
> DataBuffer.java
src/java.desktop/share/classes/java/awt/image/DataBuffer.java line 553:
> 551: if ((i < 0) || ((offset + i) < i)) {
> 552: throw new ArrayIndexOutOfBoundsException("Index cannot be
> negative : " + i);
> 553: }
The second part ensures `((offset + i) >= 0)`, doesn't it?
If the offset is negative, the exception could be confusing… because the
reported index in the message will be positive.
src/java.desktop/share/classes/java/awt/image/DataBufferByte.java line 185:
> 183: super(UNTRACKABLE, TYPE_BYTE, size, dataArray.length);
> 184: checkSize(size);
> 185: checkNumBanks(dataArray.length);
Add
Objects.requireNonNull(dataArray, "dataArray must not be null");
for consistency with the above constructors… to produce a custom message for
`NPE`?
src/java.desktop/share/classes/java/awt/image/DataBufferByte.java line 225:
> 223: checkSize(size);
> 224: checkNumBanks(dataArray.length);
> 225: Objects.requireNonNull(offsets, "offsets");
Suggestion:
Objects.requireNonNull(offsets, "offsets must not be null");
For consistency with the message for `dataArray`.
src/java.desktop/share/classes/java/awt/image/DataBufferDouble.java line 222:
> 220: Objects.requireNonNull(offsets, "offsets must not be null");
> 221: if (dataArray.length != offsets.length) {
> 222: throw new ArrayIndexOutOfBoundsException("Must be an offsets
> entry for every bank");
Should it be *“an offset entry*”? In singular.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29766#discussion_r2921036391
PR Review Comment: https://git.openjdk.org/jdk/pull/29766#discussion_r2921096672
PR Review Comment: https://git.openjdk.org/jdk/pull/29766#discussion_r2921101386
PR Review Comment: https://git.openjdk.org/jdk/pull/29766#discussion_r2921150763
