Re: RFR: (8030875) Macros for checking and returning on exceptions

2014-01-15 Thread roger riggs

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

2014-01-14 Thread roger riggs

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

2014-01-13 Thread David Holmes

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

2014-01-10 Thread roger riggs

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

2014-01-10 Thread Chris Hegarty

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

2014-01-10 Thread Alan Bateman

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

2014-01-10 Thread Michael McMahon

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

2014-01-10 Thread Mandy Chung

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

2014-01-10 Thread roger riggs

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