RFR: JDK-8013736: [launcher] cleanup code for correctness

[Kumar Srinivasan]

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 <martinrb at google.com 
> <mailto:martinrb at google.com>> 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
>     <kumar.x.srinivasan at oracle.com
>     <mailto:kumar.x.srinivasan at oracle.com>> 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
>>             == 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
>>             <kumar.x.srinivasan at oracle.com
>>             <mailto:kumar.x.srinivasan at oracle.com>> 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
>>
>>
>>
>
>
>