> 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 (*env)->GetStaticFieldID will throw a same exception if the > field cannot be found. > > Here is the new webrev: http://cr.openjdk.java.net/~dxu/8029007/webrev.01/. > Thanks!
Looks good to me Dan. -Chris. > > -Dan > > >> On 01/10/2014 10:21 AM, Mike Duigou wrote: >>> 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 >>>> are useful when I cannot decide the pending exception state by just using >>>> return values. >>>> >>>> As for the style, actually I prefer the (!pointer) to (pointer == NULL) >>>> because it is more concise and also make me avoid the typo like (pointer = >>>> NULL), which I cannot find from the compilation. Thanks! >> There's always "yoda conditions" >> https://en.wikipedia.org/wiki/Yoda_conditions, (NULL == pointer), but that's >> not likely to make anyone (besides me) happy. >> >> Mike >> >>> Not that it matters, but my preference is to == NULL. >>> >>> -Chris. >>> >>>> -Dan >>>> >>>> >>>> On 01/10/2014 08:40 AM, roger riggs wrote: >>>>> 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 do the >>>>> check and return). >>>>> >>>>> (Not a Reviewer) >>>>> >>>>> Thanks, Roger >>>>> >>>>> >>>>> >>>>> On 1/10/2014 1:31 AM, 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 >>>>>> >>>>>> -Dan >
