[9] request for review 8046777: apple.security.KeychainStore has a problem searching for identities

[Sean Mullan]

You missed one of my comments:

 >> 623-629 similar comment as above

Otherwise looks fine.

--Sean

On 11/10/2014 01:05 PM, Vincent Ryan wrote:
> Thanks Sean. I’ve generated a new webrev that addresses your comments:
>    http://cr.openjdk.java.net/~vinnie/8046777/webrev.01/
>
> On 10 Nov 2014, at 16:50, Sean Mullan <sean.mullan at oracle.com> wrote:
>
>> A few comments:
>>
>> * KeystoreImpl.m
>>
>> - it is odd to have a comment embedded in the parameter list and makes the line very long. Can you move this comment above the call?
>>
>> - update copyright year
>>
>> * KeychainStore.java
>>
>> 297-304 I think it would be cleaner to write these lines as:
>>
>>             } else {
>>                 KeyEntry ke = (KeyEntry)entry;
>>                 if (ke.chain == null || ke.chain.length == 0) {
>>                     return null;
>>                 }
>>                 return ke.chain[0];
>>             }
>>
>> 623-629 similar comment as above
>>
>> * ListKeychainStore.sh
>>
>> # See JDK-8046777
>>
>> 123,166: No need to add this comment, @bug label and source code history is sufficient.
>>
>> 156 if [ $RECOUNT -lt `expr $COUNT + 3 + 1` ]; then
>>
>> change to $COUNT + 4
>>
>> 157     echo "Error: expected >=4 private key entries in the Keychain keystores"
>>
>> $COUNT is the number of private keys in the keystore before testing, so original code is still correct: "expected >$COUNT"
>>
>> --Sean
>>
>> On 10/29/2014 07:48 PM, Vincent Ryan wrote:
>>> Please review this fix contributed by David Kocher.
>>>
>>> It corrects an issue in the Keychain keystore implementation for Mac OS where private keys
>>> configured with a key usage other than ‘any’ are not retrieved.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8046777
>>> Webrev: http://cr.openjdk.java.net/~vinnie/8046777/webrev.00/
>>>
>>> Thanks.
>>>
>