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

2014-12-02 Thread Dmitry Samersoff
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.

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

2014-12-02 Thread Staffan Larsen
These two lines are wrong. The "!= NULL” should not be there: 241 if (!(*env)->ExceptionOccurred(env) != NULL) { 255 if (!(*env)->ExceptionOccurred(env) != NULL) { Otherwise looks good. /Staffan > On 2 dec 2014, at 20:14, Dmitry Samersoff wrote: > > Please review the small fix. > > htt

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

2014-12-02 Thread Dmitry Samersoff
Staffan, Thanks! Will fix it. -Dmitry On 2014-12-02 22:51, Staffan Larsen wrote: > These two lines are wrong. The "!= NULL” should not be there: > > 241 if (!(*env)->ExceptionOccurred(env) != NULL) { > 255 if (!(*env)->ExceptionOccurred(env) != NULL) { > > Otherwise looks good. > > /St

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

2014-12-02 Thread serguei.spit...@oracle.com
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

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

2014-12-03 Thread Dmitry Samersoff
Serguei, Updated webrev http://cr.openjdk.java.net/~dsamersoff/JDK-8028773/webrev.02/ -Dmitry On 2014-12-03 01:24, serguei.spit...@oracle.com wrote: > Dmitry, > > It is good in general modulo Staffan's comments. > > There are some inconsistencies: > - the ExceptionOccurred(env) is compared

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

2014-12-03 Thread Staffan Larsen
Changes look good. What testing have you done? /Staffan > On 3 dec 2014, at 13:06, Dmitry Samersoff wrote: > > Serguei, > > Updated webrev > > http://cr.openjdk.java.net/~dsamersoff/JDK-8028773/webrev.02/ > > -Dmitry > > On 2014-12-03 01:24, serguei.spit...@oracle.com wrote: >> Dmitry, >>

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

2014-12-03 Thread Dmitry Samersoff
Staffan, Only manual smoke check: opened core jstack -v dis couple of addresses -Dmitry On 2014-12-03 16:01, Staffan Larsen wrote: > Changes look good. What testing have you done? > > /Staffan > >> On 3 dec 2014, at 13:06, Dmitry Samersoff >> wrote: >> >> Serguei, >> >> Updated webrev >> >

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

2014-12-03 Thread Staffan Larsen
Ok. > On 3 dec 2014, at 16:15, Dmitry Samersoff wrote: > > Staffan, > > Only manual smoke check: > > opened core > > jstack -v > dis couple of addresses > > -Dmitry > > On 2014-12-03 16:01, Staffan Larsen wrote: >> Changes look good. What testing have you done? >> >> /Staffan >> >>> On 3

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

2014-12-03 Thread serguei.spit...@oracle.com
Dmitry, Looks good. Thanks, Serguei On 12/3/14 4:06 AM, Dmitry Samersoff wrote: Serguei, Updated webrev http://cr.openjdk.java.net/~dsamersoff/JDK-8028773/webrev.02/ -Dmitry On 2014-12-03 01:24, serguei.spit...@oracle.com wrote: Dmitry, It is good in general modulo Staffan's comments.