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

[Sean Mullan]

On Thu, 25 Aug 2022 18:44:54 GMT, Mark Powers <mpowers at openjdk.org> wrote:

>> 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.

Right, but in this case I think if an NPE is ever thrown it would be considered a bug in the JDK because an unexpected RuntimeException would be thrown. I think requireNonNull is used more in cases where caller input is being validated and null is not valid. I find this code less readable. There are lots of cases in the JDK code where some object could theoretically be null, but it would be a bug if it was. If it was a normal case for a provider to sometimes be null here, then I would expect this code to check for null and handle it.

@valeriep is more familiar with this code, so I would also like her feedback on these changes to use requireNonNull.

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

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