On Fri, 17 Apr 2026 18:46:04 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
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8377568

Changes requested by aivanov (Reviewer).

src/java.desktop/share/classes/java/awt/image/DataBuffer.java line 169:

> 167:      *  @param numBanks the number of banks in this
> 168:      *         {@code DataBuffer}
> 169:      *  @throws IllegalArgumentException if {@code size} or {@code 
> numBanks} is less than or equal to zero.

I like the new condition better because it's symmetrical.

> ```java
>      * @throws IllegalArgumentException if {@code size} is less than or equal 
> to zero,
>      *         or {@code numBanks} is less than one.
> ```

The line is too long, it's longer than *100 chars*.

src/java.desktop/share/classes/java/awt/image/DataBuffer.java line 228:

> 226:      *         {@code DataBuffer}
> 227:      *  @param offset the offset for each bank
> 228:      *  @since 1.7

Usually, `@since` is the last statement in javadoc. Other constructors follow 
this convention.

src/java.desktop/share/classes/java/awt/image/DataBuffer.java line 287:

> 285:      *  @throws IllegalArgumentException if any element of {@code 
> offsets} is less than zero.
> 286:      *  @throws ArrayIndexOutOfBoundsException if {@code numBanks}
> 287:      *          does not equal the length of {@code offsets}

Would it be better to amend the condition for ArrayIndexOutOfBoundsException so 
that they match between `DataBuffer` and its subclasses?


     * @throws ArrayIndexOutOfBoundsException if the lengths of {@code 
dataArray}
     *          and {@code offsets} differ.

src/java.desktop/share/classes/java/awt/image/DataBufferByte.java line 91:

> 89:      * @param size The size of the banks in the {@code DataBuffer}.
> 90:      * @param numBanks The number of banks in the {@code DataBuffer}.
> 91:      * @throws IllegalArgumentException if {@code size} or {@code 
> numBanks} is less than or equal to zero

Full stop is missing in the end.

-------------

PR Review: https://git.openjdk.org/jdk/pull/29766#pullrequestreview-4146987048
PR Review Comment: https://git.openjdk.org/jdk/pull/29766#discussion_r3116803317
PR Review Comment: https://git.openjdk.org/jdk/pull/29766#discussion_r3116952491
PR Review Comment: https://git.openjdk.org/jdk/pull/29766#discussion_r3116845874
PR Review Comment: https://git.openjdk.org/jdk/pull/29766#discussion_r3116903086

Reply via email to