On Mon, 22 Aug 2022 21:45:39 GMT, Mark Powers <mpow...@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
