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

[Sean Mullan]

Looks good.

--Sean

On 11/10/2014 04:53 PM, Vincent Ryan wrote:
> I’ve updated the fix to reduce the casts to KeyEntry:
>    http://cr.openjdk.java.net/~vinnie/8046777/webrev.02/
>
>
> On 10 Nov 2014, at 18:48, Vincent Ryan <vincent.x.ryan at oracle.com> wrote:
>
>>
>> On 10 Nov 2014, at 18:13, Sean Mullan <sean.mullan at oracle.com> wrote:
>>
>>> You missed one of my comments:
>>>
>>>>> 623-629 similar comment as above
>>
>> I made the change to join the two if statements into a single one.
>> Did you want me to use a local variable instead of casts?
>>
>>>
>>> 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.
>>>>>>
>>>>
>>
>