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 <[email protected]> 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.
