Re: RFR: (8030875) Macros for checking and returning on exceptions
Hi Martin, A followup, what is the subtlety that makes it beneficial to wrap even a single statement in a do {} while (0)? It makes sense for macros with multiple statements but not for a single statement substitution of expressions. For a single statement (w/o ';'), it seems like a redundant overhead that does not solve an issue. Thanks, Roger On 1/14/2014 5:20 PM, Martin Buchholz wrote: On Tue, Jan 14, 2014 at 7:54 AM, roger riggs roger.ri...@oracle.com mailto:roger.ri...@oracle.com wrote: Hi David, The CHECK_RETURN macros have existed in java.net http://java.net for some time and I have not seen any empty statement warnings. The CHECK_EXCEPTION macros are new and does not have any uses yet. I don't plan to do any wholesale modification of current sources. The macros always produce a valid statement; (though my c/c++ may be a bit rusty). Macros should not generally be complete statements, including semicolons; then the source code looks like this: FOO() which looks (including to emacs) like someone forgot the trailing semicolon. Instead, these macros should be written using the well-known do ( ... ) while (0) idiom, e.g. #define CHECK_NULL(x) do { if ((x) == NULL) return; } while (0) (Note that it has been requested to rename the macros to include a JNU_prefix to be consistent with other macros in jni_util.h. I will propose a separate set of changes for that). Roger On 1/13/2014 9:51 PM, David Holmes wrote: Hi Roger, webrev: http://cr.openjdk.java.net/~rriggs/webrev-check-exception-8030875/ http://cr.openjdk.java.net/%7Erriggs/webrev-check-exception-8030875/ Do these macro definitions not cause empty statement warnings from the compiler? (Existing ones have the same problem I guess) Also I don't see any use of the CHECK_EXCEPTION macros? In fact the bulk of changes here seem completely unrelated to the exception aspect of this CR. Cheers, David [1] https://bugs.openjdk.java.net/browse/JDK-8030875
Re: RFR: (8030875) Macros for checking and returning on exceptions
Hi David, The CHECK_RETURN macros have existed in java.net for some time and I have not seen any empty statement warnings. The CHECK_EXCEPTION macros are new and does not have any uses yet. I don't plan to do any wholesale modification of current sources. The macros always produce a valid statement; (though my c/c++ may be a bit rusty). (Note that it has been requested to rename the macros to include a JNU_prefix to be consistent with other macros in jni_util.h. I will propose a separate set of changes for that). Roger On 1/13/2014 9:51 PM, David Holmes wrote: Hi Roger, webrev: http://cr.openjdk.java.net/~rriggs/webrev-check-exception-8030875/ Do these macro definitions not cause empty statement warnings from the compiler? (Existing ones have the same problem I guess) Also I don't see any use of the CHECK_EXCEPTION macros? In fact the bulk of changes here seem completely unrelated to the exception aspect of this CR. Cheers, David [1] https://bugs.openjdk.java.net/browse/JDK-8030875
Re: RFR: (8030875) Macros for checking and returning on exceptions
Hi Roger, On 11/01/2014 1:37 AM, roger riggs wrote: Please review: To enable native code checking consistently for thrown exceptions, the macros in net_util.h and java/util/jar/pack/coding.cpp are made consolidated and promoted to jni_util.h webrev: http://cr.openjdk.java.net/~rriggs/webrev-check-exception-8030875/ Do these macro definitions not cause empty statement warnings from the compiler? (Existing ones have the same problem I guess) Also I don't see any use of the CHECK_EXCEPTION macros? In fact the bulk of changes here seem completely unrelated to the exception aspect of this CR. Cheers, David [1] https://bugs.openjdk.java.net/browse/JDK-8030875
RFR: (8030875) Macros for checking and returning on exceptions
Please review: To enable native code checking consistently for thrown exceptions, the macros in net_util.h and java/util/jar/pack/coding.cpp are made consolidated and promoted to jni_util.h webrev: http://cr.openjdk.java.net/~rriggs/webrev-check-exception-8030875/ [1] https://bugs.openjdk.java.net/browse/JDK-8030875
Re: RFR: (8030875) Macros for checking and returning on exceptions
Thank you Roger, much appreciated. I think Dan has a change in flight that could be simplified a bit by using these. -Chris. On 10/01/14 15:37, roger riggs wrote: Please review: To enable native code checking consistently for thrown exceptions, the macros in net_util.h and java/util/jar/pack/coding.cpp are made consolidated and promoted to jni_util.h webrev: http://cr.openjdk.java.net/~rriggs/webrev-check-exception-8030875/ [1] https://bugs.openjdk.java.net/browse/JDK-8030875
Re: RFR: (8030875) Macros for checking and returning on exceptions
On 10/01/2014 15:37, roger riggs wrote: Please review: To enable native code checking consistently for thrown exceptions, the macros in net_util.h and java/util/jar/pack/coding.cpp are made consolidated and promoted to jni_util.h webrev: http://cr.openjdk.java.net/~rriggs/webrev-check-exception-8030875/ This looks okay to me. It would be good if Kumar takes a look at this too because it results in a mix of macros in the pack code. -Alan
Re: RFR: (8030875) Macros for checking and returning on exceptions
On 10/01/14 15:37, roger riggs wrote: Please review: To enable native code checking consistently for thrown exceptions, the macros in net_util.h and java/util/jar/pack/coding.cpp are made consolidated and promoted to jni_util.h webrev: http://cr.openjdk.java.net/~rriggs/webrev-check-exception-8030875/ [1] https://bugs.openjdk.java.net/browse/JDK-8030875 Looks fine to me Michael.
Re: RFR: (8030875) Macros for checking and returning on exceptions
On 1/10/2014 7:37 AM, roger riggs wrote: Please review: To enable native code checking consistently for thrown exceptions, the macros in net_util.h and java/util/jar/pack/coding.cpp are made consolidated and promoted to jni_util.h webrev: http://cr.openjdk.java.net/~rriggs/webrev-check-exception-8030875/ Looks good. Thanks for doing this Roger. Minor nit: the convention in jni.h seems to name functions that take JNIEnv* parameter with the JNU_ prefix and so might be better to rename CHECK_EXCEPTION to JNU_CheckedException. Mandy
Re: RFR: (8030875) Macros for checking and returning on exceptions
Hi Mandy, Good point; I'll create another issue to do that update. (I should have waited a bit longer for comments.) Roger On 1/10/2014 12:41 PM, Mandy Chung wrote: On 1/10/2014 7:37 AM, roger riggs wrote: Please review: To enable native code checking consistently for thrown exceptions, the macros in net_util.h and java/util/jar/pack/coding.cpp are made consolidated and promoted to jni_util.h webrev: http://cr.openjdk.java.net/~rriggs/webrev-check-exception-8030875/ Looks good. Thanks for doing this Roger. Minor nit: the convention in jni.h seems to name functions that take JNIEnv* parameter with the JNU_ prefix and so might be better to rename CHECK_EXCEPTION to JNU_CheckedException. Mandy