RFR: JDK-8291509 Minor cleanup could be done in sun.security

Mark Powers mpowers at openjdk.org
Thu Aug 25 18:47:38 UTC 2022


On Thu, 25 Aug 2022 15:00:55 GMT, Sean Mullan <mullan at openjdk.org> wrote:

>> https://bugs.openjdk.org/browse/JDK-8291509
>
> src/java.base/share/classes/sun/security/jca/ProviderList.java line 129:
> 
>> 127:         int j = 0;
>> 128:         for (ProviderConfig config : providerList.configs) {
>> 129:             if (!Objects.requireNonNull(config.getProvider()).getName().equals(name)) {
> 
> This is an unusual usage of `Objects.requireNonNull`. Is a null provider ever expected here? I don't see why this is better, the prior code will also throw NPE. Replacing `== false` with `!` is ok though.
> 
> Same comment on other cases in this file.

IntelliJ seems to think it could. The point of requireNonNull is that you control when an exception is thrown, and sooner rather than later is better. This code appears to have been working fine for a long time, so maybe NPE can't happen in practice. I'm fine with reverting this requireNonNull change here and elsewhere if you think it is unnecessary.

> src/java.base/share/classes/sun/security/jca/ProviderList.java line 679:
> 
>> 677:         private final String algorithm;
>> 678:         private final String provider;
>> 679:         private String[] alternateNames = null;
> 
> shouldn't this also be final?

If I make alternateNames final, then IJ complains "cannot assign a value to final variable".
If I make  it final and remove initialization, then make complains that alternateNames might not have been initialized.

> src/java.base/share/classes/sun/security/jca/Providers.java line 212:
> 
>> 210: 
>> 211:     // Change the thread local provider list. Use only if the current thread
>> 212:     // is already using a thread local list, and you want to change it in place.
> 
> Hmm, I'm not sure this is more grammatically correct.

I could go either way. The sentence is not that long (it's not a German sentence).  IJ is only a recommendation.

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

PR: https://git.openjdk.org/jdk/pull/9972



More information about the security-dev mailing list