Code Review Request JDK-8129988 JSSE should create a single instance of the cacerts KeyStore

Sean Mullan sean.mullan at oracle.com
Thu Jan 5 20:44:58 UTC 2017


            if ((certs != null) && descriptor.equals(temporaryDesc)) {
                 return certs;
            } else if (certs != null) {         // use new descriptor

This would be more readable as:

            if (certs != null) {
                if (descriptor.equals(temporaryDesc)) {
                    return certs;
                } else {         // use new descriptor
                    ...
                }

No need to post another webrev just for this ...

--Sean



On 1/5/17 12:32 PM, Xuelei Fan wrote:
> Thanks for the comments.
>
>    http://cr.openjdk.java.net/~xuelei/8129988/webrev.05/
>
> I re-write the getTrustedCerts() implementation.  It looks more compact
> now.
>
> Thanks,
> Xuelei
>
> On 1/5/2017 8:13 AM, Sean Mullan wrote:
>>  265             if (descriptor.equals(temporaryDesc) && (ks != null)) {
>>
>> Not sure if JVM will optimize this, but probably better to check if ks
>> != null first, and avoid calling equals unless necessary:
>>
>>  265             if (ks != null && descriptor.equals(temporaryDesc)) {
>>
>> Also, similar comment for getTrustedCerts -- you should first check if
>> csRef.get() is null.
>> If it is null, then you don't have to check the TrustStoreDescriptor,
>> you can go straight to
>> reloading the keystore and getting the certs.
>>
>> No other comments.
>>
>> --Sean
>>
>> On 1/4/2017 8:17 PM, Xuelei Fan wrote:
>>> New webrev:
>>>
>>> http://cr.openjdk.java.net/~xuelei/8129988/webrev.04/
>>


More information about the security-dev mailing list