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