RFR: 8263436: Silly array comparison in GaloisCounterMode.overlapDetection
Anthony Scarpino
ascarpino at openjdk.java.net
Thu Mar 11 18:18:07 UTC 2021
On Thu, 11 Mar 2021 17:55:39 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:
>> Can you explain why the silly way it is written now is any different than what you are proposing? When running jshell your proposed change returns the same result as the existing code:
>>
>> one ==> byte[5] { 1, 2, 3, 4, 5 }
>> two ==> byte[5] { 1, 2, 3, 4, 5 }
>> jshell> one != two
>> $9 ==> true
>>
>> jshell> !one.equals(two)
>> $10 ==> true
>>
>> jshell> one != one
>> $11 ==> false
>>
>> jshell> !one.equals(one)
>> $12 ==> false
>>
>> Is the analysis tool thinking equals() is comparing the contents?
>
>> Can you explain why the silly way it is written now is any different than what you are proposing? Is the analysis tool thinking equals() is comparing the contents?
>
> See: https://rules.sonarsource.com/java/RSPEC-2159.
>
> Using `equals` raises the question if actual array contents comparisons was implied (which would require `Arrays.equals`). I know it is not implied here. Using `==` makes the intent clear. This is why the rule asks to choose either `==` or `Arrays.equals`. This is the only instance where that rule triggers in whole OpenJDK code.
>
> From a micro-optimization perspective, `equals` is a method. This means interpreters are would have to do that call. Compilers would hopefully inline the call, but then it would still imply the null-check, because NPE should be thrown if one side is `null`.
>
> So, why call `equals()`, when `==` is better?
meh
-------------
PR: https://git.openjdk.java.net/jdk/pull/2938
More information about the security-dev
mailing list