RE: RFR: 8170544: Fix code scan findings in libnet

2016-12-29 Thread Lindenmaier, Goetz
Hi Christoph, Thanks for the fixes, looks good now! Best regards, Goetz. > -Original Message- > From: Langer, Christoph > Sent: Donnerstag, 29. Dezember 2016 14:21 > To: Lindenmaier, Goetz > Cc: net-dev@openjdk.java.net; Chris Hegarty

RE: RFR: 8170544: Fix code scan findings in libnet

2016-12-29 Thread Langer, Christoph
Hi Goetz, thanks for reviewing this. I have addressed your comments in a new webrev: http://cr.openjdk.java.net/~clanger/webrevs/8170544.1/ Here's the details: > http://cr.openjdk.java.net/~clanger/webrevs/8170544.0/src/java.base/share/na > tive/libnet/net_util.c.udiff.html > + * check

RE: [8u-dev]: Request for Review and Approval: 8075484: SocketInputStream.socketRead0 can hang even with soTimeout set

2016-12-29 Thread Langer, Christoph
Hi Vyom, generally, I think it is a good idea to have a look at NET_GetCurrentTime vs. JVM_CurrentTimeMillis again. However, NET_GetCurrentTime is self-contained and does not need arguments whereas JVM_CurrentTimeMillis needs a JNI env pointer. And since NET_GetCurrentTime is called from

Re: [8u-dev]: Request for Review and Approval: 8075484: SocketInputStream.socketRead0 can hang even with soTimeout set

2016-12-29 Thread Vyom Tewari
Hi Christoph, As this is not direct backport what about using the existing function "JVM_CurrentTimeMillis(env, 0);" instead of "NET_GetCurrentTime() ". When i did this code change i did not know that we already have this.If you use the existing one then you can simply remove the

RE: RFR: 8170544: Fix code scan findings in libnet

2016-12-29 Thread Lindenmaier, Goetz
Hi Christoph, I had a look at your change. It's a bit confusing that you call the variable 'sa' everywhere, as a field of it has the same name. sa.sa looks confusing. But it's good that you name it everywhere the same, and that you went through all this code. Some details:

[8u-dev]: Request for Review and Approval: 8075484: SocketInputStream.socketRead0 can hang even with soTimeout set

2016-12-29 Thread Langer, Christoph
Hi, please review (and eventually approve) the change for downporting 8075484. Webrev for 8u-dev: http://cr.openjdk.java.net/~clanger/webrevs/8075484.8udev/ Bug: https://bugs.openjdk.java.net/browse/JDK-8075484 JDK9 Change: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/af17b6bc08dd JDK9 Review