RFR : JDK-8046545 launcher fix to check function return values

Neil Toda neil.toda at oracle.com
Mon Jul 21 16:31:31 UTC 2014


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