RFR(S), SA, warnings from b116 for hotspot.agent.src.share.native: JNI exception pending

Staffan Larsen staffan.larsen at oracle.com
Wed Dec 3 13:01:48 UTC 2014


Changes look good. What testing have you done?

/Staffan

> On 3 dec 2014, at 13:06, Dmitry Samersoff <dmitry.samersoff at oracle.com> wrote:
> 
> Serguei,
> 
> Updated webrev
> 
> http://cr.openjdk.java.net/~dsamersoff/JDK-8028773/webrev.02/
> 
> -Dmitry
> 
> On 2014-12-03 01:24, serguei.spitsyn at oracle.com wrote:
>> Dmitry,
>> 
>> It is good in general modulo Staffan's comments.
>> 
>> There are some inconsistencies:
>> -  the ExceptionOccurred(env) is compared to != NULL (which make the
>> logic more complex)
>>     in some places and no such comparison (implicit comparison instead)
>> in others
>> - two different forms of check/action are used:
>>      -  (*env)->ExceptionOccurred(env) and
>>      -  !(*env)->ExceptionOccurred(env)
>> 
>> I'd suggest to do it the same way and always return the "default" value
>> if an exception occurred.
>> It will make the logic more flat and clear.
>> Otherwise, we have to think what exact value is returned in exception
>> occurred cases like at lines 241, 255.
>> 
>> The comment // OOM is used inconsistently too.
>> 
>> 
>> Thanks,
>> Serguei
>> 
>> 
>> On 12/2/14 11:14 AM, Dmitry Samersoff wrote:
>>> Please review the small fix.
>>> 
>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8028773/webrev.01/
>>> 
>>> Added more missed exception checks to sadis.c
>>> 
>> 
> 
> 
> -- 
> Dmitry Samersoff
> Oracle Java development team, Saint Petersburg, Russia
> * I would love to change the world, but they won't give me the sources.



More information about the serviceability-dev mailing list