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 I added comments to a few previous comments that are marked resolved, and GitHub hides them, and I can't unresolve the threads. [**Comment 1**](https://github.com/openjdk/jdk/pull/29766#discussion_r2927673622): This is confusing now. You said, > I guess there was no point in the explicit check for dataArray == null > because the dataArray.length expression above implicitly does it. I see no > value in trying to do anything to have the explicit check first, so I've > removed all the explicit checks that follow an implicit check. So, implicit check is good enough for `dataArray`, but it's not good enough for the offsets array. The custom message could be then more helpful and read `"offsets must not be null"` (instead of just the word `"offsets"`) as I suggested. Huh, you referred to the two-dimensional `dataArray` that's used in a call to super-constructor. You can add a custom message there too: since JDK 22, statements before call to `super()` are allowed. [**Comment 2**](https://github.com/openjdk/jdk/pull/29766#discussion_r2927678584): Well, if you want a custom message you can add it: public DataBufferByte(byte[][] dataArray, int size) { Objects.requireNonNull(dataArray, "dataArray must not be null"); super(UNTRACKABLE, TYPE_BYTE, size, dataArray.length); This is allowed since JDK 22. [**Comment 3**](https://github.com/openjdk/jdk/pull/29766#discussion_r2927695320): Yeah, I'm on the fence here. I like it to be succinct. But I would be utterly confused if I get an exception saying Index cannot be negative : 1 if the offset is -2 for example. Thus, more verbose code and clear error message is better. ------------- PR Comment: https://git.openjdk.org/jdk/pull/29766#issuecomment-4050594574
