On Wed, 11 Mar 2026 22:31:00 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: > > 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? 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. 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? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29766#discussion_r2927616873 PR Review Comment: https://git.openjdk.org/jdk/pull/29766#discussion_r2927549803 PR Review Comment: https://git.openjdk.org/jdk/pull/29766#discussion_r2927554225
