Here is the modified webrev: http://cr.openjdk.java.net/~ksrini/8013736/webrev.1/
delta webrev to the last webrev: http://cr.openjdk.java.net/~ksrini/8013736/webrev.1/webrev.delta/index.html I added the do { ..... } while (0) pattern to all the launcher's macro defs, I also addressed Alan's comments. I know that the native unpacker uses macros which will need this pattern I will fix that separately. Alan, do you want me to search and file bugs on other jdk components ? Thanks Kumar
Oops. Of course, that shouldn't have the trailing semicolon #define NULL_CHECK_OR_RETURN(ncor_pointer_to_check, \ ncor_failure_return_value) \ do { \ if ((ncor_pointer_to_check) == NULL) { \ JLI_ReportErrorMessage(JNI_ERROR); \ return ncor_failure_return_value; \ } \ } while(0)On Thu, May 2, 2013 at 11:16 AM, Martin Buchholz <[email protected] <mailto:[email protected]>> wrote:What would Martin actually do? #define NULL_CHECK_OR_RETURN(ncor_pointer_to_check, \ ncor_failure_return_value) \ do { \ if ((ncor_pointer_to_check) == NULL) { \ JLI_ReportErrorMessage(JNI_ERROR); \ return ncor_failure_return_value; \ } \ } while(0); On Thu, May 2, 2013 at 10:45 AM, Kumar Srinivasan <[email protected] <mailto:[email protected]>> wrote: 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 == NULLYes 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_eYou want me to rename the macro parameter e to N_C_RV_e ? or something else say ncrve to avoid collision ? KumarOn 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
