2d reviewer please . Re: RFR : JDK-8046545 launcher fix to check function return values

[Neil Toda]

Sometimes a bit too cryptic.  Sorry all for the confusion.

-neil

On 7/23/2014 5:22 PM, Kevin Rushforth wrote:
> Yeah, my brain is so wired to seeing 2d or 3d and thinking graphics, 
> that I naturally assumed be meant 2D team...
>
> -- Kevin
>
>
> Phil Race wrote:
>> I suspect Neil meant "2nd" reviewer. I can't see any 2D content here.
>>
>> BTW Kumar - if there is a Contributed-by: I think jcheck will be OK with
>> you as committer and reviewer, however a 2nd reviewer is a very good 
>> idea in general
>>
>> -phil.
>>
>> On 7/23/14 4:06 PM, Kevin Rushforth wrote:
>>> I might suggest Phil Race or Andrew Brygin, but you could also just 
>>> send e-mail to 2d-dev at openjdk.java.net and ask for a volunteer (I am 
>>> not a Reviewer for 2D).
>>>
>>> -- Kevin
>>>
>>>
>>> Neil Toda wrote:
>>>>
>>>> I'm hoping some one will volunteer to be a 2d reviewer so we can 
>>>> satisfy jcheck requirement for a 2d
>>>> review for this 8u patch.
>>>>
>>>> It is a very simple set of macros and a couple of applications that 
>>>> we hope to use going forward
>>>> in making sure that JNI exceptions are caught in the launcher.
>>>>
>>>> http://cr.openjdk.java.net/~ntoda/8046545/webrev-02/
>>>>
>>>> Thanks
>>>>
>>>> -neil
>>>>
>>>> On 7/21/2014 9:31 AM, Neil Toda wrote:
>>>>>
>>>>> Hi Kumar
>>>>>
>>>>> Actually, the null check macros have different parameters. 
>>>>> NCRV_return_value is an integer.
>>>>> RETURNVALUE in CHECK_JNI_RETURN is a macro,  which allows us to 
>>>>> have only the two macros:
>>>>> CHECK_JNI_RETURN
>>>>> and
>>>>> CHECK_JNI_RETRUN_EXCEPTION
>>>>>
>>>>> I also think it is cleaner since there are only two, and they are 
>>>>> for JNI, to keep them self contained.
>>>>>
>>>>> Would someone be willing to review webrev-02, which contain 
>>>>> Kumar's suggested change in the
>>>>> comments included with the macros.
>>>>>
>>>>> Thanks
>>>>>
>>>>> -neil
>>>>>
>>>>>
>>>>> On 7/19/2014 8:02 AM, Kumar Srinivasan wrote:
>>>>>> [ Since I am sponsoring this patch, I think jcheck needs one more 
>>>>>> Reviewer besides myself]
>>>>>>
>>>>>> Neil,
>>>>>>
>>>>>> looking at your webrev: 
>>>>>> http://cr.openjdk.java.net/~ntoda/8046545/webrev-02/
>>>>>>
>>>>>> Can we not re-use the existing macro for null check ?
>>>>>>
>>>>>> #define NULL_CHECK_RETURN_VALUE(NCRV_check_pointer, 
>>>>>> NCRV_return_value)
>>>>>>
>>>>>> so thus your new macro would become....
>>>>>>
>>>>>> +#define CHECK_JNI_RETURN(JNIRETURN, RETURNVALUE) \
>>>>>> +    CHECK_JNI_RETURN_EXCEPTION(RETURNVALUE); \
>>>>>> -    do { \
>>>>>> -        if ((JNIRETURN) == NULL) { \
>>>>>> -            JLI_ReportErrorMessage(JNI_ERROR); \
>>>>>> -            RETURNVALUE; \
>>>>>> -        } \
>>>>>> -    } while (JNI_FALSE)
>>>>>> +    NULL_CHECK_RETURN_VALUE(JNI_RETURN, RETURN_VALUE);
>>>>>>
>>>>>>
>>>>>> Kumar
>>>>>>
>>>>>> On 7/18/2014 10:40 AM, Neil Toda wrote:
>>>>>>>
>>>>>>> Thanks Kumar.  Yes, misspoke here.  Will correct the comment.
>>>>>>>
>>>>>>> On 7/18/2014 10:35 AM, Kumar Srinivasan wrote:
>>>>>>>> Neil,
>>>>>>>> The fix looks good. However there is an inaccuracy in the comment:
>>>>>>>>
>>>>>>>> + *  Normally, JNI calls do not return if an exception is thrown.
>>>>>>>> + *  However, this behavior can change in the future,
>>>>>>>> + *  so check for thrown exceptions.
>>>>>>>>
>>>>>>>> This is not true, JNI calls *will* return if an exception is 
>>>>>>>> thrown, however best
>>>>>>>> JNI practices dictate that a pending Exception(s) must be 
>>>>>>>> cleared or caught, before
>>>>>>>> attempting another JNI call. Under such circumstances the 
>>>>>>>> return value will usually
>>>>>>>> be an error or a null value. I suggest making this change to 
>>>>>>>> reflect this.
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>> Kumar
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 7/18/2014 9:53 AM, Neil Toda wrote:
>>>>>>>>>
>>>>>>>>> Please review this launcher change.
>>>>>>>>>
>>>>>>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8046545
>>>>>>>>> webrev: http://cr.openjdk.java.net/~ntoda/8046545/webrev-01/
>>>>>>>>>
>>>>>>>>> Summary:
>>>>>>>>>
>>>>>>>>> Introduce a set of macros for launcher to be used to check for 
>>>>>>>>> certain conditions after
>>>>>>>>> return from select functions.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>