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

[Martin Buchholz]

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>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> 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> wrote:
>>>
>>>> Hi,
>>>>
>>>> Please review simple fixes for code correctness in the launcher.
>>>>
>>>> http://cr.openjdk.java.net/~ksrini/8013736/webrev.0/
>>>>
>>>> Thanks
>>>> Kumar
>>>>
>>>>
>>>
>>
>>
>