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. >>>>> >>> >
