Re: RFR: 8376297: ArrayIndexOutOfBoundsException Not Documented for SinglePixelPackedSampleModel.getSampleSize(int) [v3]
On Mon, 2 Feb 2026 20:01:19 GMT, Phil Race wrote: >> Update the specification of concrete SampleModel classes which over-ride >> getSampleSize(int band) to describe how the behave. >> It isn't entirely pretty because 2 of them ignore the band parameter and >> always have .. > > Phil Race has updated the pull request incrementally with one additional > commit since the last revision: > > 8376297 Marked as reviewed by aivanov (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/29457#pullrequestreview-3746933966
Re: RFR: 8376297: ArrayIndexOutOfBoundsException Not Documented for SinglePixelPackedSampleModel.getSampleSize(int) [v2]
On Fri, 30 Jan 2026 13:00:34 GMT, Alexey Ivanov wrote:
>> Phil Race has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 8376297
>
> test/jdk/java/awt/image/GetSampleSizeTest.java line 47:
>
>> 45: ComponentSampleModel csm =
>> 46: new ComponentSampleModel(DataBuffer.TYPE_BYTE,
>> 47: width, height, 1, width, bandOffsets);
>
> I still think the lines 46–47 should be indented by 4 more spaces: either is
> a continuation line of the line above.
>
>
> Suggestion:
>
> ComponentSampleModel csm =
> new ComponentSampleModel(DataBuffer.TYPE_BYTE,
> width, height, 1, width, bandOffsets);
>
>
> However, the indentation is somewhat consistent now: lines 58 and 69 are also
> indented by 4 spaces only. Yet they should be indented by 8 spaces.
I don't know where this 8 spaces rule came from and I thought you meant it for
the 2nd or later continuation line, not for the first.
My preferred style is to always indent 4 more except I like to line up the arg
lists vertically.
> test/jdk/java/awt/image/GetSampleSizeTest.java line 75:
>
>> 73: throw new RuntimeException("Unexpected numBands");
>> 74: }
>> 75: try {
>
> Suggestion:
>
> if (numBands != 4) {
> throw new RuntimeException("Unexpected numBands");
> }
> try {
>
> The brace that closes the `if` block is still indented incorrectly, it should
> be in the same column as `i` and `t` of `if` above and `try` below.
ok
-
PR Review Comment: https://git.openjdk.org/jdk/pull/29457#discussion_r2755945238
PR Review Comment: https://git.openjdk.org/jdk/pull/29457#discussion_r2755944985
Re: RFR: 8376297: ArrayIndexOutOfBoundsException Not Documented for SinglePixelPackedSampleModel.getSampleSize(int) [v3]
> Update the specification of concrete SampleModel classes which over-ride > getSampleSize(int band) to describe how the behave. > It isn't entirely pretty because 2 of them ignore the band parameter and > always have .. Phil Race has updated the pull request incrementally with one additional commit since the last revision: 8376297 - Changes: - all: https://git.openjdk.org/jdk/pull/29457/files - new: https://git.openjdk.org/jdk/pull/29457/files/ebde5c8a..ea3124dd Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=29457&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=29457&range=01-02 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/29457.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/29457/head:pull/29457 PR: https://git.openjdk.org/jdk/pull/29457
Re: RFR: 8376297: ArrayIndexOutOfBoundsException Not Documented for SinglePixelPackedSampleModel.getSampleSize(int) [v3]
On Thu, 29 Jan 2026 18:50:42 GMT, Alexey Ivanov wrote:
>> Phil Race has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 8376297
>
> test/jdk/java/awt/image/GetSampleSizeTest.java line 1:
>
>> 1: /*
>
> I suggest creating `test-` methods rather than using a code block inside the
> `main` method and call the methods from main, it would make the code better
> structured and convey the intent clearer.
I don't think it needs multiple methods.
I didn't really need to do even the { .. } but it made it easier to make sure
of not re-using the var names.
> test/jdk/java/awt/image/GetSampleSizeTest.java line 47:
>
>> 45: ComponentSampleModel csm =
>> 46: new ComponentSampleModel(DataBuffer.TYPE_BYTE,
>> 47: width, height, 1, width, bandOffsets);
>
> Suggestion:
>
> ComponentSampleModel csm =
> new ComponentSampleModel(DataBuffer.TYPE_BYTE,
> width, height, 1, width, bandOffsets);
>
> Indent the continuation lines by 8 spaces which is the standard indentation
> for continuation lines.
ok
> test/jdk/java/awt/image/GetSampleSizeTest.java line 59:
>
>> 57: MultiPixelPackedSampleModel mppsm =
>> 58: new MultiPixelPackedSampleModel(DataBuffer.TYPE_BYTE,
>> 59: width, height, 4);
>
> Suggestion:
>
> MultiPixelPackedSampleModel mppsm =
> new MultiPixelPackedSampleModel(DataBuffer.TYPE_BYTE,
> width, height, 4);
>
> Indent the continuation lines by 8 spaces
I removed the line break instead
> test/jdk/java/awt/image/GetSampleSizeTest.java line 72:
>
>> 70: new SinglePixelPackedSampleModel(DataBuffer.TYPE_BYTE,
>> 71: width, height, bitMask);
>> 72: int numBands = sppsm.getNumBands();
>
> Suggestion:
>
> SinglePixelPackedSampleModel sppsm =
> new SinglePixelPackedSampleModel(DataBuffer.TYPE_BYTE,
> width, height, bitMask);
> int numBands = sppsm.getNumBands();
>
> Indent the continuation lines by 8 spaces; `numBands` isn't a continuation
> line, and it should align to `SinglePixelPackedSampleModel` declaration.
fixed int numBands, removed line break from the constructor parameter list
> test/jdk/java/awt/image/GetSampleSizeTest.java line 76:
>
>> 74: if (numBands != 4) {
>> 75: throw new RuntimeException("Unexpected numBands");
>> 76: }
>
> Suggestion:
>
> if (numBands != 4) {
> throw new RuntimeException("Unexpected numBands");
> }
>
> Misaligned closing brace.
fixed
-
PR Review Comment: https://git.openjdk.org/jdk/pull/29457#discussion_r2743954454
PR Review Comment: https://git.openjdk.org/jdk/pull/29457#discussion_r2743950893
PR Review Comment: https://git.openjdk.org/jdk/pull/29457#discussion_r2743946838
PR Review Comment: https://git.openjdk.org/jdk/pull/29457#discussion_r2743944431
PR Review Comment: https://git.openjdk.org/jdk/pull/29457#discussion_r2743940310
Re: RFR: 8376297: ArrayIndexOutOfBoundsException Not Documented for SinglePixelPackedSampleModel.getSampleSize(int) [v2]
On Thu, 29 Jan 2026 23:36:08 GMT, Phil Race wrote:
>> Update the specification of concrete SampleModel classes which over-ride
>> getSampleSize(int band) to describe how the behave.
>> It isn't entirely pretty because 2 of them ignore the band parameter and
>> always have ..
>
> Phil Race has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8376297
Changes requested by aivanov (Reviewer).
test/jdk/java/awt/image/GetSampleSizeTest.java line 47:
> 45: ComponentSampleModel csm =
> 46: new ComponentSampleModel(DataBuffer.TYPE_BYTE,
> 47: width, height, 1, width, bandOffsets);
I still think the lines 46–47 should be indented by 4 more spaces: either is a
continuation line of the line above.
Suggestion:
ComponentSampleModel csm =
new ComponentSampleModel(DataBuffer.TYPE_BYTE,
width, height, 1, width, bandOffsets);
However, the indentation is somewhat consistent now: lines 58 and 69 are also
indented by 4 spaces only. Yet they should be indented by 8 spaces.
test/jdk/java/awt/image/GetSampleSizeTest.java line 75:
> 73: throw new RuntimeException("Unexpected numBands");
> 74: }
> 75: try {
Suggestion:
if (numBands != 4) {
throw new RuntimeException("Unexpected numBands");
}
try {
The brace that closes the `if` block is still indented incorrectly, it should
be in the same column as `i` and `t` of `if` above and `try` below.
-
PR Review: https://git.openjdk.org/jdk/pull/29457#pullrequestreview-3728155545
PR Review Comment: https://git.openjdk.org/jdk/pull/29457#discussion_r2746158825
PR Review Comment: https://git.openjdk.org/jdk/pull/29457#discussion_r2746164592
Re: RFR: 8376297: ArrayIndexOutOfBoundsException Not Documented for SinglePixelPackedSampleModel.getSampleSize(int) [v2]
> Update the specification of concrete SampleModel classes which over-ride > getSampleSize(int band) to describe how the behave. > It isn't entirely pretty because 2 of them ignore the band parameter and > always have .. Phil Race has updated the pull request incrementally with one additional commit since the last revision: 8376297 - Changes: - all: https://git.openjdk.org/jdk/pull/29457/files - new: https://git.openjdk.org/jdk/pull/29457/files/b936635c..ebde5c8a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=29457&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=29457&range=00-01 Stats: 9 lines in 2 files changed: 0 ins; 2 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/29457.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/29457/head:pull/29457 PR: https://git.openjdk.org/jdk/pull/29457
Re: RFR: 8376297: ArrayIndexOutOfBoundsException Not Documented for SinglePixelPackedSampleModel.getSampleSize(int)
On Tue, 27 Jan 2026 22:51:27 GMT, Phil Race wrote:
> Update the specification of concrete SampleModel classes which over-ride
> getSampleSize(int band) to describe how the behave.
> It isn't entirely pretty because 2 of them ignore the band parameter and
> always have ..
Changes requested by aivanov (Reviewer).
src/java.desktop/share/classes/java/awt/image/MultiPixelPackedSampleModel.java
line 242:
> 240: * this method ignores the {@code band} parameter and returns
> 241: * the sample size of the single band.
> 242: * @param band the specified band (ignored).
Suggestion:
* @param band the specified band (ignored)
For consistency with other methods which don't have the full stop in
description of parameters.
test/jdk/java/awt/image/GetSampleSizeTest.java line 1:
> 1: /*
I suggest creating `test-` methods rather than using a code block inside the
`main` method and call the methods from main, it would make the code better
structured and convey the intent clearer.
test/jdk/java/awt/image/GetSampleSizeTest.java line 47:
> 45: ComponentSampleModel csm =
> 46: new ComponentSampleModel(DataBuffer.TYPE_BYTE,
> 47: width, height, 1, width, bandOffsets);
Suggestion:
ComponentSampleModel csm =
new ComponentSampleModel(DataBuffer.TYPE_BYTE,
width, height, 1, width, bandOffsets);
Indent the continuation lines by 8 spaces which is the standard indentation for
continuation lines.
test/jdk/java/awt/image/GetSampleSizeTest.java line 59:
> 57: MultiPixelPackedSampleModel mppsm =
> 58: new MultiPixelPackedSampleModel(DataBuffer.TYPE_BYTE,
> 59: width, height, 4);
Suggestion:
MultiPixelPackedSampleModel mppsm =
new MultiPixelPackedSampleModel(DataBuffer.TYPE_BYTE,
width, height, 4);
Indent the continuation lines by 8 spaces
test/jdk/java/awt/image/GetSampleSizeTest.java line 72:
> 70: new SinglePixelPackedSampleModel(DataBuffer.TYPE_BYTE,
> 71: width, height, bitMask);
> 72: int numBands = sppsm.getNumBands();
Suggestion:
SinglePixelPackedSampleModel sppsm =
new SinglePixelPackedSampleModel(DataBuffer.TYPE_BYTE,
width, height, bitMask);
int numBands = sppsm.getNumBands();
Indent the continuation lines by 8 spaces; `numBands` isn't a continuation
line, and it should align to `SinglePixelPackedSampleModel` declaration.
test/jdk/java/awt/image/GetSampleSizeTest.java line 76:
> 74: if (numBands != 4) {
> 75: throw new RuntimeException("Unexpected numBands");
> 76: }
Suggestion:
if (numBands != 4) {
throw new RuntimeException("Unexpected numBands");
}
Misaligned closing brace.
-
PR Review: https://git.openjdk.org/jdk/pull/29457#pullrequestreview-3724404777
PR Review Comment: https://git.openjdk.org/jdk/pull/29457#discussion_r2743009235
PR Review Comment: https://git.openjdk.org/jdk/pull/29457#discussion_r2743039318
PR Review Comment: https://git.openjdk.org/jdk/pull/29457#discussion_r2743019932
PR Review Comment: https://git.openjdk.org/jdk/pull/29457#discussion_r2743024796
PR Review Comment: https://git.openjdk.org/jdk/pull/29457#discussion_r2743029588
PR Review Comment: https://git.openjdk.org/jdk/pull/29457#discussion_r2743031578
Re: RFR: 8376297: ArrayIndexOutOfBoundsException Not Documented for SinglePixelPackedSampleModel.getSampleSize(int)
On Tue, 27 Jan 2026 22:51:27 GMT, Phil Race wrote: > Update the specification of concrete SampleModel classes which over-ride > getSampleSize(int band) to describe how the behave. > It isn't entirely pretty because 2 of them ignore the band parameter and > always have .. Looks good - Marked as reviewed by kizune (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/29457#pullrequestreview-3723921376
Re: RFR: 8376297: ArrayIndexOutOfBoundsException Not Documented for SinglePixelPackedSampleModel.getSampleSize(int)
On Tue, 27 Jan 2026 22:51:27 GMT, Phil Race wrote: > Update the specification of concrete SampleModel classes which over-ride > getSampleSize(int band) to describe how the behave. > It isn't entirely pretty because 2 of them ignore the band parameter and > always have .. Marked as reviewed by azvegint (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/29457#pullrequestreview-3723419051
Re: RFR: 8376297: ArrayIndexOutOfBoundsException Not Documented for SinglePixelPackedSampleModel.getSampleSize(int)
On Tue, 27 Jan 2026 22:51:27 GMT, Phil Race wrote: > Update the specification of concrete SampleModel classes which over-ride > getSampleSize(int band) to describe how the behave. > It isn't entirely pretty because 2 of them ignore the band parameter and > always have .. The CSR is ready for review : https://bugs.openjdk.org/browse/JDK-8376608 - PR Comment: https://git.openjdk.org/jdk/pull/29457#issuecomment-3813474439
Re: RFR: 8376297: ArrayIndexOutOfBoundsException Not Documented for SinglePixelPackedSampleModel.getSampleSize(int)
On Tue, 27 Jan 2026 22:51:27 GMT, Phil Race wrote: > Update the specification of concrete SampleModel classes which over-ride > getSampleSize(int band) to describe how the behave. > It isn't entirely pretty because 2 of them ignore the band parameter and > always have .. Marked as reviewed by serb (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/29457#pullrequestreview-3718662931
Re: RFR: 8376297: ArrayIndexOutOfBoundsException Not Documented for SinglePixelPackedSampleModel.getSampleSize(int)
On Wed, 28 Jan 2026 00:17:54 GMT, Chen Liang wrote: > I recommend specifying `IndexOutOfBoundsException` instead of AIOOBE: this > allows implementation freedom in case an underlying storage is at some point > changed to a `List` or some other non-array structure. This package has 152 specified cases of @throws ArrayIndexOutOfBoundsException and just one of * @throws IndexOutOfBoundsException and I don't think we are likely to change the implementation anyway. So I would prefer to leave it as it is. - However counting this I noticed one un-specified exception on an un-related class in the package. But that should be handled separately. - PR Comment: https://git.openjdk.org/jdk/pull/29457#issuecomment-3813040105
Re: RFR: 8376297: ArrayIndexOutOfBoundsException Not Documented for SinglePixelPackedSampleModel.getSampleSize(int)
On Tue, 27 Jan 2026 22:51:27 GMT, Phil Race wrote: > Update the specification of concrete SampleModel classes which over-ride > getSampleSize(int band) to describe how the behave. > It isn't entirely pretty because 2 of them ignore the band parameter and > always have .. I recommend specifying `IndexOutOfBoundsException` instead of AIOOBE: this allows implementation freedom in case an underlying storage is at some point changed to a `List` or some other non-array structure. - PR Comment: https://git.openjdk.org/jdk/pull/29457#issuecomment-3808236881
