Re: JDK 9 RFR of JDK-8036747: Fix unchecked lint warnings in java.security.Provider

2014-03-05 Thread Xuelei Fan
Looks fine to me. Thanks, Joe! Xuelei On 3/6/2014 9:35 AM, Joe Darcy wrote: > Hello, > > Please review the patch below which addresses > > JDK-8036747: Fix unchecked lint warnings in java.security.Provider > > In brief, java.security.Provider extends Properties which in extends > Hashtabl

JDK 9 RFR of JDK-8036747: Fix unchecked lint warnings in java.security.Provider

2014-03-05 Thread Joe Darcy
Hello, Please review the patch below which addresses JDK-8036747: Fix unchecked lint warnings in java.security.Provider In brief, java.security.Provider extends Properties which in extends Hashtable even though morally Properties are a Map. The implementations of new-in-JDK-8 methods li

Re: Review Request for JDK-8030114: [parfait] warnings from b119 for jdk.src.share.native.sun.security.smartcardio: JNI exception pending

2014-03-05 Thread Valerie (Yu-Ching) Peng
I still need a reviewer for integrating the changes for 8030114... Any taker? Thanks, Valerie On 02/21/14 16:41, Valerie (Yu-Ching) Peng wrote: Good points, I have updated the webrev accordingly. The current callers of Java_sun_security_smartcardio_PCSC_SCardGetStatusChange seems to only p

Re: Code review request: 8036543 Parfait JNI pending exceptions for j2secmod.c, j2secmod_md.c, and p11_md.c

2014-03-05 Thread Anthony Scarpino
Sure.. I debated that piece of code before the review too. Tony On 03/05/2014 03:52 PM, Valerie (Yu-Ching) Peng wrote: line 133 - 138, I think it's better to take the ReleaseStringUTFChars() call of 'configDir' out of the block for 'functionName'. So we can ensure that both are released even

Re: Code review request: 8036543 Parfait JNI pending exceptions for j2secmod.c, j2secmod_md.c, and p11_md.c

2014-03-05 Thread Valerie (Yu-Ching) Peng
line 133 - 138, I think it's better to take the ReleaseStringUTFChars() call of 'configDir' out of the block for 'functionName'. So we can ensure that both are released even if the code from 82-94 are somehow later altered. The rest looks fine. Thanks, Valerie On 03/04/14 22:56, Anthony Sca

Re: Review Request for JDK-8033571: [parfait] warning from b128 for security/smartcardio/pcsc_md.c: JNI exception pending

2014-03-05 Thread Valerie (Yu-Ching) Peng
I just feel that using ExceptionCheck call (than null), it's more obvious that there is already a pending exception. So, for me, it offers more clarity, comparing to checking for null which some JNI calls return when errors are encountered (however, there may be no pending exceptions at least

Re: Review Request for JDK-8033571: [parfait] warning from b128 for security/smartcardio/pcsc_md.c: JNI exception pending

2014-03-05 Thread Anthony Scarpino
On 02/13/2014 03:29 PM, Valerie (Yu-Ching) Peng wrote: Can someone please review the fixes which checks for pending exceptions in native code "pcsc_md.c"? The fix is trivial scope-wise. Webrev: http://cr.openjdk.java.net/~valeriep/8033571/webrev.00/ Thanks, Valerie In my change that's out fo

Re: Code review request, 8036676, Rename class name testEnabledProtocols to TestEnabledProtocols

2014-03-05 Thread Wang Weijun
Change looks fine. That was an awkward name. --Max On Mar 5, 2014, at 21:06, Xuelei Fan wrote: > Hi, > > Please review this simple test fix: > >http://cr.openjdk.java.net/~xuelei/8036676/webrev.00/ > > Updated to use capital letter to start a class name. > > Thanks, > Xuelei

Code review request, 8036676, Rename class name testEnabledProtocols to TestEnabledProtocols

2014-03-05 Thread Xuelei Fan
Hi, Please review this simple test fix: http://cr.openjdk.java.net/~xuelei/8036676/webrev.00/ Updated to use capital letter to start a class name. Thanks, Xuelei

Re: CipherInputStream for AEAD modes is insecure (GCM, etc.): ciphertext tampering without detection possible

2014-03-05 Thread Xuelei Fan
I think the implementation of GCM cipher should respect the specification of GCM mode. Before authentication tag get checked, no plaintext should be released to application. See section 7.0 of GCM spec (NIST SP-800-38D). Xuelei On 3/5/2014 5:56 AM, Philipp Heckel wrote: > Thank you all for your