On 5/2/2013 10:17 AM, Martin Buchholz wrote:
This is global fix creep, but ...

:(


these macros are also found in the hotspot sources.
I would rewrite all the macros in the jdk to adopt the blessed style
do { ... } while(0)
and remove all comments in the jdk of the form
 /* next token must be ; */
If you want a macro that does nothing at all, you should define it
do {} while (0)
instead of defining it to the empty string.
I am not following, could you be more explicit on how this applies to

-#define NULL_CHECK0(e) if ((e) == 0) { \
+#define NULL_CHECK_RV(e, rv) if ((e) == 0) { \
     JLI_ReportErrorMessage(JNI_ERROR); \
-    return 0; \
+    return rv; \
   }
-#define NULL_CHECK(e) if ((e) == 0) { \
-    JLI_ReportErrorMessage(JNI_ERROR); \
-    return; \
-  }
+#define NULL_CHECK0(e) NULL_CHECK_RV(e, 0)
+#define NULL_CHECK(e) NULL_CHECK_RV(e, )
+




    I would also be inclined to change
    == 0
    to
    == NULL


Yes will take care of this, as well as Alan suggestion added a check for malloc return.

    This seems like another occasion to use the weird

    do { ... } while(0)

    trick to make the macro behave more like a statement.

    I might obfuscate the macro parameters to make collisions less
    likely, e.g. e => N_C_RV_e


You want me to rename the macro parameter e to N_C_RV_e ? or something else
say ncrve to avoid collision ?

Kumar





    On Wed, May 1, 2013 at 12:33 PM, Kumar Srinivasan
    <[email protected]
    <mailto:[email protected]>> wrote:

        Hi,

        Please review simple fixes for code correctness in the launcher.

        http://cr.openjdk.java.net/~ksrini/8013736/webrev.0/
        <http://cr.openjdk.java.net/%7Eksrini/8013736/webrev.0/>

        Thanks
        Kumar




Reply via email to