Please, review a modified fix: http://cr.openjdk.java.net/~dsamersoff/JDK-8028773/webrev.03/
Windows compiler doesn't allow declaration in the middle of the function for c code. Also I put my few cents to reduce build noise and add a pragma that disable windows compiler warnings that have no value in this context. -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 <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. > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
