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.

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



Reply via email to