Changes look good. What testing have you done? /Staffan
> On 3 dec 2014, at 13:06, Dmitry Samersoff <dmitry.samers...@oracle.com> 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. >> >> 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.
