Re: RFR: JDK-8029007, Check src/share/native/sun/misc code for JNI pending exceptions

2014-01-11 Thread Chris Hegarty
On 10 Jan 2014, at 19:40, Dan Xu dan...@oracle.com wrote: Thank you for all the feedback. I have updated my changes to use CHECK_EXCEPTION_RETURN and CHECK_EXCEPTION macro recently added into jni_util.h. I also removed else block in function setStaticIntField() in Version.c since

Re: RFR: JDK-8029007, Check src/share/native/sun/misc code for JNI pending exceptions

2014-01-10 Thread Alan Bateman
On 10/01/2014 06:31, Dan Xu wrote: Hi All, Please review the fix for JNI pending exception issues reported in jdk-8029007. Thanks! Webrev: http://cr.openjdk.java.net/~dxu/8029007/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8029007 This looks good, the only one that isn't clear

Re: RFR: JDK-8029007, Check src/share/native/sun/misc code for JNI pending exceptions

2014-01-10 Thread roger riggs
Hi Dan, Just pushed are macros in jni_util.h to do the same function as your new macros. Please update to use the common macros instead of introducing new ones. Style wise, I would avoid mixing binary operators (!) with pointers. it is clearer to compare with NULL. (The CHECK_NULL macro will

Re: RFR: JDK-8029007, Check src/share/native/sun/misc code for JNI pending exceptions

2014-01-10 Thread Dan Xu
Thanks, Chris. Right, I don't find any usage of getThreadStateValues, so it is simpler to just remove it. -Dan On 01/09/2014 11:49 PM, Chris Hegarty wrote: Looks ok to me. I presume getThreadStateValues is simply no longer needed. -Chris. On 10 Jan 2014, at 06:31, Dan Xu

Re: RFR: JDK-8029007, Check src/share/native/sun/misc code for JNI pending exceptions

2014-01-10 Thread Dan Xu
On 01/10/2014 02:32 AM, Alan Bateman wrote: On 10/01/2014 06:31, Dan Xu wrote: Hi All, Please review the fix for JNI pending exception issues reported in jdk-8029007. Thanks! Webrev: http://cr.openjdk.java.net/~dxu/8029007/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8029007

Re: RFR: JDK-8029007, Check src/share/native/sun/misc code for JNI pending exceptions

2014-01-10 Thread roger riggs
Hi Dan, One other comment, instead of changing the return type of the setStaticIntField just return and the caller should check for exceptions and return. See jni.h: CHECK_EXCEPTION(env) Roger On 1/10/2014 11:40 AM, roger riggs wrote: Hi Dan, Just pushed are macros in jni_util.h to do the

Re: RFR: JDK-8029007, Check src/share/native/sun/misc code for JNI pending exceptions

2014-01-10 Thread Dan Xu
Hi Roger, My macro is a little different from yours, which compares with -1 instead of NULL. I also see CHECK_EXCEPTION macro. Thanks for adding them, which are useful when I cannot decide the pending exception state by just using return values. As for the style, actually I prefer the

Re: RFR: JDK-8029007, Check src/share/native/sun/misc code for JNI pending exceptions

2014-01-10 Thread Chris Hegarty
On 10 Jan 2014, at 18:05, Dan Xu dan...@oracle.com wrote: Hi Roger, My macro is a little different from yours, which compares with -1 instead of NULL. I also see CHECK_EXCEPTION macro. Thanks for adding them, which are useful when I cannot decide the pending exception state by just using

Re: RFR: JDK-8029007, Check src/share/native/sun/misc code for JNI pending exceptions

2014-01-10 Thread Dan Xu
Yes, you are right. I just found this macro. It looks very handy to use. Thanks! -Dan On 01/10/2014 09:59 AM, roger riggs wrote: Hi Dan, One other comment, instead of changing the return type of the setStaticIntField just return and the caller should check for exceptions and return. See

Re: RFR: JDK-8029007, Check src/share/native/sun/misc code for JNI pending exceptions

2014-01-10 Thread Mike Duigou
On Jan 10 2014, at 10:09 , Chris Hegarty chris.hega...@oracle.com wrote: On 10 Jan 2014, at 18:05, Dan Xu dan...@oracle.com wrote: Hi Roger, My macro is a little different from yours, which compares with -1 instead of NULL. I also see CHECK_EXCEPTION macro. Thanks for adding them, which

Re: RFR: JDK-8029007, Check src/share/native/sun/misc code for JNI pending exceptions

2014-01-10 Thread Dan Xu
Thank you for all the feedback. I have updated my changes to use CHECK_EXCEPTION_RETURN and CHECK_EXCEPTION macro recently added into jni_util.h. I also removed else block in function setStaticIntField() in Version.c since (*env)-GetStaticFieldID will throw a same exception if the field cannot

Re: RFR: JDK-8029007, Check src/share/native/sun/misc code for JNI pending exceptions

2014-01-09 Thread Chris Hegarty
Looks ok to me. I presume getThreadStateValues is simply no longer needed. -Chris. On 10 Jan 2014, at 06:31, Dan Xu dan...@oracle.com wrote: Hi All, Please review the fix for JNI pending exception issues reported in jdk-8029007. Thanks! Webrev: