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 |
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 |
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 |
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 |
In reply to this post by 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. Marked as reviewed by azvegint (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/3125 |
In reply to this post by Sergey Bylokhov-2
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 |
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 |
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 |
In reply to this post by 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 |
Free forum by Nabble | Edit this page |