Yeah, my brain is so wired to seeing 2d or 3d and thinking graphics, that I naturally assumed be meant 2D team...

-- Kevin


Phil Race wrote:
I suspect Neil meant "2nd" reviewer. I can't see any 2D content here.

BTW Kumar - if there is a Contributed-by: I think jcheck will be OK with
you as committer and reviewer, however a 2nd reviewer is a very good idea in general

-phil.

On 7/23/14 4:06 PM, Kevin Rushforth wrote:
I might suggest Phil Race or Andrew Brygin, but you could also just send e-mail to 2d-...@openjdk.java.net and ask for a volunteer (I am not a Reviewer for 2D).

-- Kevin


Neil Toda wrote:

I'm hoping some one will volunteer to be a 2d reviewer so we can satisfy jcheck requirement for a 2d
review for this 8u patch.

It is a very simple set of macros and a couple of applications that we hope to use going forward
in making sure that JNI exceptions are caught in the launcher.

http://cr.openjdk.java.net/~ntoda/8046545/webrev-02/

Thanks

-neil

On 7/21/2014 9:31 AM, Neil Toda wrote:

Hi Kumar

Actually, the null check macros have different parameters. NCRV_return_value is an integer. RETURNVALUE in CHECK_JNI_RETURN is a macro, which allows us to have only the two macros:
CHECK_JNI_RETURN
and
CHECK_JNI_RETRUN_EXCEPTION

I also think it is cleaner since there are only two, and they are for JNI, to keep them self contained.

Would someone be willing to review webrev-02, which contain Kumar's suggested change in the
comments included with the macros.

Thanks

-neil


On 7/19/2014 8:02 AM, Kumar Srinivasan wrote:
[ Since I am sponsoring this patch, I think jcheck needs one more Reviewer besides myself]

Neil,

looking at your webrev: http://cr.openjdk.java.net/~ntoda/8046545/webrev-02/

Can we not re-use the existing macro for null check ?

#define NULL_CHECK_RETURN_VALUE(NCRV_check_pointer, NCRV_return_value)

so thus your new macro would become....

+#define CHECK_JNI_RETURN(JNIRETURN, RETURNVALUE) \
+    CHECK_JNI_RETURN_EXCEPTION(RETURNVALUE); \
-    do { \
-        if ((JNIRETURN) == NULL) { \
-            JLI_ReportErrorMessage(JNI_ERROR); \
-            RETURNVALUE; \
-        } \
-    } while (JNI_FALSE)
+    NULL_CHECK_RETURN_VALUE(JNI_RETURN, RETURN_VALUE);


Kumar

On 7/18/2014 10:40 AM, Neil Toda wrote:

Thanks Kumar.  Yes, misspoke here.  Will correct the comment.

On 7/18/2014 10:35 AM, Kumar Srinivasan wrote:
Neil,
The fix looks good. However there is an inaccuracy in the comment:

+ *  Normally, JNI calls do not return if an exception is thrown.
+ *  However, this behavior can change in the future,
+ *  so check for thrown exceptions.

This is not true, JNI calls *will* return if an exception is thrown, however best JNI practices dictate that a pending Exception(s) must be cleared or caught, before attempting another JNI call. Under such circumstances the return value will usually be an error or a null value. I suggest making this change to reflect this.

Thanks
Kumar



On 7/18/2014 9:53 AM, Neil Toda wrote:

Please review this launcher change.

Issue: https://bugs.openjdk.java.net/browse/JDK-8046545
webrev: http://cr.openjdk.java.net/~ntoda/8046545/webrev-01/

Summary:

Introduce a set of macros for launcher to be used to check for certain conditions after
return from select functions.








Reply via email to