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

Sean Mullan mullan at openjdk.org
Thu Aug 25 15:11:28 UTC 2022


On Mon, 22 Aug 2022 21:45:39 GMT, Mark Powers <mpowers at openjdk.org> wrote:

> https://bugs.openjdk.org/browse/JDK-8291509

Some initial comments so far.

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.

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?

src/java.base/share/classes/sun/security/jca/Providers.java line 104:

> 102:      * Start JAR verification. This sets a special provider list for
> 103:      * the current thread. You MUST save the return value from this
> 104:      * method, and you MUST call stopJarVerification() with that object

Hmm, I'm not sure this is more grammatically correct.

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.

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

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



More information about the security-dev mailing list