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

Neil Toda neil.toda at oracle.com
Wed Jul 23 22:50:41 UTC 2014


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




More information about the core-libs-dev mailing list