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.r...@oracle.com> wrote:
On 10 Nov 2014, at 18:13, Sean Mullan <sean.mul...@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.mul...@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.
