On Wed, 24 Jan 2024 15:41:11 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> Alexey Bakhtin has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Add KeychainStore-ROOT keystore for root certificates
>
> src/java.base/macosx/classes/apple/security/AppleProvider.java line 89:
>
>> 87: "KeychainStore",
>> "apple.security.KeychainStore.USER"));
>> 88: putService(new ProviderService(p, "KeyStore",
>> 89: "KeychainStore-ROOT",
>> "apple.security.KeychainStore.ROOT"));
>
> I think the correct class names should be `KeychainStore$USER` and
> `KeychainStore$ROOT`, although they might be used.
Thank you. updated with the $ sign
> src/java.base/macosx/classes/apple/security/KeychainStore.java line 52:
>
>> 50: */
>> 51:
>> 52: abstract class KeychainStore extends KeyStoreSpi {
>
> Make it `abstract sealed`.
Good catch. Thank you. Added `sealed`
> src/java.base/macosx/classes/apple/security/KeychainStore.java line 216:
>
>> 214: */
>> 215: private String getName()
>> 216: {
>
> Put the closing brace to the previous line. Actually, probably not worth
> creating a new method.
OK. Not much reason for this method. removed
> src/java.base/macosx/classes/apple/security/KeychainStore.java line 546:
>
>> 544:
>> 545: synchronized(entries) {
>> 546:
>
> Useless.
Fixed
> src/java.base/macosx/native/libosxsecurity/KeystoreImpl.m line 478:
>
>> 476: // Load user trustSettings into inputTrust
>> 477: if (SecTrustSettingsCopyTrustSettings(certRef, domain,
>> &trustSettings) == errSecSuccess && trustSettings != NULL) {
>> 478: if(inputTrust == NULL) {
>
> Add a space after `if`.
Fixed
> src/java.base/macosx/native/libosxsecurity/KeystoreImpl.m line 544:
>
>> 542:
>> 543: // Read Trust Anchors
>> 544: if(SecTrustCopyAnchorCertificates(&currAnchors) == errSecSuccess) {
>
> Add a space after `if`.
Fixed
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16722#discussion_r1467006056
PR Review Comment: https://git.openjdk.org/jdk/pull/16722#discussion_r1467006192
PR Review Comment: https://git.openjdk.org/jdk/pull/16722#discussion_r1467006360
PR Review Comment: https://git.openjdk.org/jdk/pull/16722#discussion_r1467006453
PR Review Comment: https://git.openjdk.org/jdk/pull/16722#discussion_r1467006522
PR Review Comment: https://git.openjdk.org/jdk/pull/16722#discussion_r1467006582
