RFR: 8263981: java.awt.image.ComponentSampleModel equals/hashcode use numBands twice

[Aleksey Shipilev-5]

SonarCloud reports the problem in ComponentSampleModel.equals:
  Correct one of the identical sub-expressions on both sides of operator "&&"

...near "this.numBands == that.numBands". It is checked twice. hashCode also processes it twice.

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

Commit messages:
 - Update copyright line
 - JDK-8263981: java.awt.image.ComponentSampleModel equals/hashcode use numBands twice

Changes: https://git.openjdk.java.net/jdk/pull/3125/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3125&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8263981
  Stats: 4 lines in 1 file changed: 0 ins; 3 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3125.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3125/head:pull/3125

PR: https://git.openjdk.java.net/jdk/pull/3125

[Sergey Bylokhov-2]

On Mon, 22 Mar 2021 15:55:16 GMT, Aleksey Shipilev <[hidden email]> wrote:

> SonarCloud reports the problem in ComponentSampleModel.equals:
>   Correct one of the identical sub-expressions on both sides of operator "&&"
>
> ...near "this.numBands == that.numBands". It is checked twice. hashCode also processes it twice.

Marked as reviewed by serb (Reviewer).

src/java.desktop/share/classes/java/awt/image/ComponentSampleModel.java line 1228:

> 1226:         }
> 1227:         hash ^= numBands;
> 1228:         hash <<= 8;

Since this mistake was done in two places I think this is a typo here and not an intentional thing.

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

PR: https://git.openjdk.java.net/jdk/pull/3125

[Aleksey Shipilev-5]

On Mon, 22 Mar 2021 17:44:03 GMT, Sergey Bylokhov <[hidden email]> wrote:

>> SonarCloud reports the problem in ComponentSampleModel.equals:
>>   Correct one of the identical sub-expressions on both sides of operator "&&"
>>
>> ...near "this.numBands == that.numBands". It is checked twice. hashCode also processes it twice.
>
> src/java.desktop/share/classes/java/awt/image/ComponentSampleModel.java line 1228:
>
>> 1226:         }
>> 1227:         hash ^= numBands;
>> 1228:         hash <<= 8;
>
> Since this mistake was done in two places I think this is a typo here and not an intentional thing.

I am a bit confused by this comment :) I suspect that `equals` got duplicated `numBands` first, and then whoever did the `hashcode` just hashed the same fields in the same order, duplicating it again.

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

PR: https://git.openjdk.java.net/jdk/pull/3125

[Sergey Bylokhov-2]

On Mon, 22 Mar 2021 17:48:21 GMT, Aleksey Shipilev <[hidden email]> wrote:

>> src/java.desktop/share/classes/java/awt/image/ComponentSampleModel.java line 1228:
>>
>>> 1226:         }
>>> 1227:         hash ^= numBands;
>>> 1228:         hash <<= 8;
>>
>> Since this mistake was done in two places I think this is a typo here and not an intentional thing.
>
> I am a bit confused by this comment :) I suspect that `equals` got duplicated `numBands` first, and then whoever did the `hashcode` just hashed the same fields in the same order, duplicating it again.

I hope so, and not in the opposite order. I'll try to check the history f this file.

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

PR: https://git.openjdk.java.net/jdk/pull/3125

[Alexander Zvegintsev-2]

On Mon, 22 Mar 2021 15:55:16 GMT, Aleksey Shipilev <[hidden email]> wrote:

> SonarCloud reports the problem in ComponentSampleModel.equals:
>   Correct one of the identical sub-expressions on both sides of operator "&&"
>
> ...near "this.numBands == that.numBands". It is checked twice. hashCode also processes it twice.

Marked as reviewed by azvegint (Reviewer).

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

PR: https://git.openjdk.java.net/jdk/pull/3125

[Aleksey Shipilev-5]

On Mon, 22 Mar 2021 18:01:49 GMT, Sergey Bylokhov <[hidden email]> wrote:

>> I am a bit confused by this comment :) I suspect that `equals` got duplicated `numBands` first, and then whoever did the `hashcode` just hashed the same fields in the same order, duplicating it again.
>
> I hope so, and not in the opposite order. I'll try to check the history f this file.

Please do. I'll wait for a bit.

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

PR: https://git.openjdk.java.net/jdk/pull/3125

[Sergey Bylokhov-2]

On Tue, 23 Mar 2021 06:50:26 GMT, Aleksey Shipilev <[hidden email]> wrote:

>> I hope so, and not in the opposite order. I'll try to check the history f this file.
>
> Please do. I'll wait for a bit.

I have found this bug, and the fix for it just implemented both methods at once, so this is a typo.
https://bugs.openjdk.java.net/browse/JDK-4430788

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

PR: https://git.openjdk.java.net/jdk/pull/3125

[Aleksey Shipilev-5]

On Tue, 23 Mar 2021 22:17:56 GMT, Sergey Bylokhov <[hidden email]> wrote:

>> Please do. I'll wait for a bit.
>
> I have found this bug, and the fix for it just implemented both methods at once, so this is a typo.
> https://bugs.openjdk.java.net/browse/JDK-4430788

Brilliant, thanks. I'll integrate now.

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

PR: https://git.openjdk.java.net/jdk/pull/3125

[Aleksey Shipilev-5]

On Mon, 22 Mar 2021 15:55:16 GMT, Aleksey Shipilev <[hidden email]> wrote:

> SonarCloud reports the problem in ComponentSampleModel.equals:
>   Correct one of the identical sub-expressions on both sides of operator "&&"
>
> ...near "this.numBands == that.numBands". It is checked twice. hashCode also processes it twice.

This pull request has now been integrated.

Changeset: cb776edf
Author:    Aleksey Shipilev <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/cb776edf
Stats:     4 lines in 1 file changed: 0 ins; 3 del; 1 mod

8263981: java.awt.image.ComponentSampleModel equals/hashcode use numBands twice

Reviewed-by: serb, azvegint

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

PR: https://git.openjdk.java.net/jdk/pull/3125