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

[Kevin Rushforth]

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.
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>