RFR: (8031737) rename jni_util.h macros for checking and returning on exceptions

roger riggs roger.riggs at oracle.com
Thu Jan 16 19:51:09 UTC 2014


Hi Kumar,

The parameter names are purely local to the macro. They do not need to 
be unique.
If the macro was longer (though it is now a lot longer than the original),
it might make the code more readable.
(Though I'm sure someone has a different convention).

Roger


On 1/16/2014 1:57 PM, Kumar Srinivasan wrote:
> Roger,
>
> one more thing, shouldn't the parameters be unique ?
> I think Martin had me do this for all macros in the java  launcher
> for example please see this changeset, I recently pushed.
> http://hg.openjdk.java.net/jdk9/dev/jdk/rev/6c50c972a101
>
> Kumar
>
> On 1/16/2014 9:30 AM, Kumar Srinivasan wrote:
>> Hi Roger,
>>
>> Its confusing to use a JNU_ prefixed macro on a method not involvng jni,
>> why not rename these to modulo JNU_ ?
>>
>> I am cc'ing Alex as he has a related bug fix in his queue for pack's 
>> jni code.
>>
>> Kumar
>>
>>> Hi Alan,
>>>
>>> The macros are generally useful even without being used on a method 
>>> that
>>> involves jni.  An overly aggressive find caught the uses in 
>>> java/util/jar/pack.
>>>
>>> But yes, it might be better to limit their scope to functions that 
>>> involve jni.
>>>
>>> Roger
>>>
>>>
>>> On 1/16/2014 11:41 AM, Alan Bateman wrote:
>>>> On 16/01/2014 16:26, roger riggs wrote:
>>>>> Please review:
>>>>>
>>>>> The native macros for checking and returning when exceptions
>>>>> are thrown have been renamed to include the "JNU_" prefix
>>>>> consistent with other functions in jni_util.h.
>>>>>
>>>>> The macros have been renamed and existing uses in the jdk repository
>>>>> for networking, pack200, and have been updated. A jprt run has 
>>>>> passed (except for unrelated failures).
>>>>>
>>>>> webrev:
>>>>> http://cr.openjdk.java.net/~rriggs/webrev-jnu-check-rename-8031737/
>>>> Did you mean to rename the CHECK_NULL and CHECK_NULL_RETURN macros? 
>>>> They don't require a JNIEnv so I'm wondering if JNU_* really make 
>>>> sense here.
>>>>
>>>> -Alan.
>>>
>>
>




More information about the core-libs-dev mailing list