Hi Arthur,
Changes looks good to me, one minor comment, in HTTPTestServer.java we can
avoid new local variable (InetAddress address) .
Thanks,
Vyom
On Tue, Mar 12, 2019 at 10:30 PM Arthur Eubanks wrote:
> Thanks for creating the umbrella bug.
>
> Split long line.
> Added the HTTPTestServer.j
Hi Chris,
Change looks good to me.
Thanks,
Vyom
On Tue, Mar 19, 2019 at 4:12 PM Chris Hegarty
wrote:
> This review request is to resolve a documentation regression in Java 9,
> resulting from part of the changes for 8132478, where {@value
> java.net.URLConnection#contentPathProp} was inadverten
Hi Ivan,
Looks OK to me, in case of exception "ni" will be null you can use the
macro(CHECK_NULL_RETURN) if you are not clearing the JNI exception, this
will save at least one additional function( "(*env)->ExceptionOccurred(env)"
) call.
Thanks,
Vyom
On Wed, Mar 20, 2019 at 9:49 PM Ivan Geras
Hi Ivan,
thanks for explanation, code change looks good to me.
Thanks,
Vyom
On Thu, Mar 21, 2019 at 11:26 AM Ivan Gerasimov
wrote:
> Thanks Vyom!
>
> On 3/20/19 10:13 PM, Vyom Tiwari wrote:
>
> Hi Ivan,
>
> Looks OK to me, in case of exception "ni" will b
Looks good to me.
Vyom
On Tue, May 7, 2019 at 7:20 PM Michael McMahon
wrote:
> Could I get the following small change reviewed please?
> The recent change in the SocketImpl code unmasked the fact that
> the protected constructor of ServerSocket was not checking for a null
> impl parameter as req
Hi Daniel,
Changes looks good to me, as you said code is copied from one test to
another, i found FtpGetContent.java where same FtpServer code is copied.
Are you planning to fix FtpGetContent.java as well ?.
Thanks,
Vyom
On Wed, May 15, 2019 at 2:18 PM Daniel Fuchs
wrote:
> Hi Arthue,
>
> On
Hi Daniel,
latest change looks good to me.
Even i wanted to remove duplicate "FtpServer code" that we had copy and
pasted but somehow I did not got time do it.
Thanks,
Vyom
On Wed, May 15, 2019 at 7:08 PM Daniel Fuchs
wrote:
> Hi Vyom,
>
> On 15/05/2019 11:25, Vyom T
Hi Aleks,
latest changes looks good to me .
Thanks,
Vyom
On Wed, May 15, 2019 at 11:12 PM Aleks Efimov
wrote:
> Hi Daniel,
>
> Thanks for the review. I've modified Socket_getInputStream_[read|write]
> to follow your suggestion:
> -ServerSocket ss = new ServerSocket(0);
>
Hi Arthur,
do we need "Integer.toString(4)" to convert int to string in
SocksProxyVersion ?
Thanks,
Vyom
On Fri, May 17, 2019 at 4:46 AM Arthur Eubanks wrote:
> bug: https://bugs.openjdk.java.net/browse/JDK-8224081
> webrev: http://cr.openjdk.java.net/~aeubanks/8224081/webrev.00/index.html
>
> T
+1
On Fri, May 17, 2019 at 3:29 PM Daniel Fuchs
wrote:
> Hi Arthur,
>
> On 17/05/2019 00:16, Arthur Eubanks wrote:
> > bug: https://bugs.openjdk.java.net/browse/JDK-8224081
> > webrev:
> http://cr.openjdk.java.net/~aeubanks/8224081/webrev.00/index.html
> >
> > Tests that try to use SOCKS v4 will
Hi Daniel,
Overall changes looks good to me, please update the copy write date that
you missed in couple of files. In "TestHttpServer.java" we are printing
error message on System.err and throwing the RuntimeException both as
below, do you think we need both ?
System.err.println ("Server could n
sage ("Server
could not start:") on System.err as well ?
Thanks,
Vyom
On Fri, May 24, 2019 at 7:32 PM Daniel Fuchs
wrote:
> Hi Vyom,
>
> On 24/05/2019 14:36, Vyom Tiwari wrote:
> > Hi Daniel,
> > Overall changes looks good to me, please update the copy write date tha
Hi Jaikiran,
Code changes look OK to me , couple of minor comments
1-> you don't need "this" to access stop(this.stop)
2-> i think super.stop() should be the first line in the overridden stop
method.
3-> it is matter of personal preference but my preference is use the new
variable name 'running' t
Hi Daniel,
Code change looks good to me.
Thanks,
Vyom
On Wed, Oct 23, 2019 at 10:50 PM Daniel Fuchs
wrote:
> Hi,
>
> Please find below a trivial test fix for:
>
> 8231631: sun/net/ftp/FtpURLConnectionLeak.java fails intermittently
> with NPE
> https://bugs.openjdk.java.net/browse/JDK
Hi Ravi/Daniel,
At my local env(REL 7) test is passing without fix as well. Although my
local repo contain some additional code changes but it is not related with
the current fix.
Test1 Passed with: Connect timed out
Test2 Passed with: Connect timed out
##
Please
Hi All,
Please find the below fix which resolves the issue(
https://bugs.openjdk.java.net/browse/JDK-8238579).
"HttpURLConnection.writeRequests()" retry in case of any write failure,
during retry it creates new HttpsClient without connectTimeout &
readTimeout. Below fix sets the connect & read
Hi Daniel/Michael,
Please find the latest webrev(
http://cr.openjdk.java.net/~vtewari/8237858/webrev0.2/index.html), where i
put a test case as well.
Thanks,
Vyom
On Wed, Mar 11, 2020 at 3:16 PM Bernd Eckenfels
wrote:
> Hello,
>
> Depending on the test environment you don't need much native too
Looks good to me.
Vyom
On Fri, Mar 20, 2020 at 12:49 PM Ivan Gerasimov
wrote:
> Hello!
>
> An OOM error is intermittently observed on MacOS when joining a
> multicast group.
>
> A workaround was implemented as part of the fix for JDK-8236925.
>
> It is proposed to backport a portion of that fix,
Hi Vladimir,
In LinuxSocketOptions.c we have lot's of duplicate code, can you please
reuse "socketOptionSupported" & "handleError" as follows, this we remove
lot's of duplicate code.
Thanks,
Vyom
diff -r b6b4506a7827 src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c
--- a/src/jdk.net/linux/
Hi Vladimir,
changes looks much better now, i still see that
function(IncomingNapiIdSupported) & variable(IncomingNapiIdOptSupported)
does not follow Java naming convention.
private static final boolean IncomingNapiIdOptSupported =+
platformSocketOptions.IncomingNapiIdSupported();
Thanks,
Hi Vladimir,
Latest webrev(03) looks ok to me , one minor bit, method signature(Signature:
(I)I;) is not correct in incomingNapiIdSupported0.
Thanks,
Vyom
++/*+ * Class: jdk_net_LinuxSocketOptions+ * Method:
incomingNapiIdSupported0+ * Signature: (I)I;+ */+JNIEXPORT jboolean
JNICALL Java_j
Looks good to me, minor comment can you please use some int constant in
place of (65527, 65507) ?.
Vyom
On Fri, May 8, 2020 at 7:19 PM Patrick Concannon <
patrick.concan...@oracle.com> wrote:
> Hi,
>
> Could someone please review my fix for JDK-8242885
> 'PlainDatagramSocketImpl doesn’t allow fo
Hi Vladimir,
Latest changes looks good to me, but i am not 100% sure if we need to
distinguish between "read only socket option" and "socket option not
supported" as below
if (!incomingNapiIdOptSupported)+throw new
UnsupportedOperationException("Attempt to set unsupported
Thanks for the clarification Alan. I think, i missed some part of
discussion.
Vyom
On Tue, May 12, 2020 at 1:20 PM Alan Bateman
wrote:
> On 12/05/2020 08:34, Vyom Tiwari wrote:
>
> Hi Vladimir,
>
> Latest changes looks good to me, but i am not 100% sure if we need to
> disting
Hi Aleksei,
Latest webrev(01) looks good to me, one minor comment "PREFER_IPV4_STACK"
variable naming convention is not consistent with other similar
variables(preferIPv6Address) in InetAddress.java.
Thanks,
Vyom
On Mon, May 25, 2020 at 4:18 PM Aleks Efimov
wrote:
> Hi Alan, Daniel,
>
> Thank
Hi Patrick,
Changes looks good to me.
Thanks,
Vyom
On Wed, May 27, 2020 at 9:12 PM Patrick Concannon <
patrick.concan...@oracle.com> wrote:
> Hi,
>
> Could someone please review my webrev and CSR for JDK-8244582 'Remove
> terminally deprecated Solaris-specific SO_FLOW_SLA socket option'?
>
> Th
020 05:21, Vyom Tiwari wrote:
> > Hi Daniel/Michael,
> >
> > Please find the latest
> > webrev(http://cr.openjdk.java.net/~vtewari/8237858/webrev0.2/index.html),
>
> > where i put a test case as well.
> >
> Can you look at make/test/JtregNativeJdk.gmk? That s
Hi,
Please find the latest webrev(
http://cr.openjdk.java.net/~vtewari/8237858/webrev0.4/index.html) where i
fix the build issue as well.
Thanks,
Vyom
On Wed, Jun 17, 2020 at 3:34 PM Vyom Tiwari wrote:
> Hi Alan,
>
> Please find the latest webrev(
> http://cr.openjdk.java.net/~vte
==
>
> For the second @run - the test failed in timeout:
>
> run main/othervm/native SocketAcceptInterruptTest
> --System.out:(3/107)--
> Sending SIGPIPE to ServerSocket thread.
> Sending close Signal to server thread...
> Timeout refired
wrote:
> On 24/06/2020 05:53, Vyom Tiwari wrote:
> > Hi Daniel/Alan,
> >
> > Thanks for the review , I was not sure how JVM/JDK handles the signals
> > that's why I installed my own signal handler. As you both pointed out,
> > I removed the custom signal handle
uld
> be used (for instance CountDownLatch) than relying on arbitrary
> timeout. In particular `isSet` could be a CountDownLatch.
>
> best regards,
>
> -- daniel
>
> On 24/06/2020 05:53, Vyom Tiwari wrote:
> > Hi Daniel/Alan,
> >
> > Thanks for the review , I was not
let me refactor the test, i will try to incorporate your suggestion.
Vyom
On Wed, Jun 24, 2020 at 5:21 PM Daniel Fuchs
wrote:
> On 24/06/2020 12:14, Vyom Tiwari wrote:
> > 3-> Looking at SocketAcceptInterruptTest it looks like the test
> > will succeed if it manages to sen
Hi Daniel/Alan,
Please find the latest webrev(
http://cr.openjdk.java.net/~vtewari/8237858/webrev0.6/index.html).
Thanks,
Vyom
On Wed, Jun 24, 2020 at 5:32 PM Vyom Tiwari wrote:
> let me refactor the test, i will try to incorporate your suggestion.
> Vyom
>
>
> On Wed, Jun 24,
Hi Bernd,
When we added these socket options to JDK, that time these options were not
available on Windows.
Thanks,
Vyom
On Fri, Jun 26, 2020 at 7:12 AM Bernd Eckenfels
wrote:
> Hello,
>
> This would be a great addition. I do not understand why it does not
> support the options available for
Hi Severin,
Overall code changes looks ok to me, I build & tested at my local REL it
worked fine for me.
Below are few minor comments.
1-> Net.java
1.1-> I think you don't need to explicitly convert value to integer and
then pass it. You can avoid the local int variable creation as follows.
think webrev is ignoring the spaces that is why you are observing the
formatting issues in (linux/bsd)_close.c files. In my local repo code looks
well formatted.
Thanks,
Vyom
On Sat, Jun 27, 2020 at 11:46 AM Alan Bateman
wrote:
> On 26/06/2020 05:17, Vyom Tiwari wrote:
> > Hi Daniel/Alan,
&
of the test. If tests send signals in
separate threads then tests have to make sure that server thread is alive
and running.
Thanks,
Vyom
On Sat, Jun 27, 2020 at 12:56 PM Vyom Tiwari wrote:
> Hi Alan,
>
> Thanks for the review comment, we can send multiple signals for sure. I
> w
do let me know if i am missing something.
Thanks,
Vyom
On Mon, Jun 29, 2020 at 2:25 PM Severin Gehwolf wrote:
> Hi Vyom!
>
> On Fri, 2020-06-26 at 15:55 +0530, Vyom Tiwari wrote:
> > Hi Severin,
> >
> > Overall code changes looks ok to me, I build & tested at my
< 0) {
+NET_ERROR(env, JNU_JAVANETPKG "SocketException", "socket
closed");
+return -1;
+} else {
+jint optval, rv;
+socklen_t sz = sizeof (optval);
+rv = getsockopt(fd, SOCK_OPT_LEVEL, TCP_KEEPINTVL, &optval,
Hi Daniel,
Thanks for review please find the latest webrev(
http://cr.openjdk.java.net/~vtewari/8237858/webrev0.8/index.html). I fixed
the Windows build issue.
Thanks,
Vyom
On Mon, Jun 29, 2020 at 10:23 PM Daniel Fuchs
wrote:
> Hi Vyom,
>
>
> On 29/06/2020 08:03, Vyom Tiwari w
address all your review comments.
Thanks,
Vyom
On Sat, Jul 4, 2020 at 11:41 AM Alan Bateman
wrote:
> On 30/06/2020 06:39, Vyom Tiwari wrote:
> >
> >
> > Thanks for review please find the latest
> > webrev(http://cr.openjdk.java.net/~vtewari/8237858/webrev0.8/index.html)
Hi Martin
Thanks for the review, I will try to address your review comment.
I wanted to write a simple test case for this issue but it is getting more
complex.
Thanks,
Vyom
On Sat, Jul 4, 2020 at 8:14 PM Martin Buchholz wrote:
> On Fri, Jul 3, 2020 at 11:12 PM Alan Bateman
> wrote:
>
> > - "s
gt; by PoolCleaner and use it in many tests.
>
> On Sat, Jul 4, 2020 at 8:24 AM Vyom Tiwari wrote:
> >
> > Hi Martin
> > Thanks for the review, I will try to address your review comment.
> >
> > I wanted to write a simple test case for this issue but it is gettin
1:09:20,621Z]
> T:\workspace\open\test\jdk\java\net\Socket\libNativeThread.c(54) : warning
> C4716: 'Java_NativeThread_signal': must return a value
>
> Kind regards,
> Patrick
>
> On 7 Jul 2020, at 04:14, Vyom Tiwari wrote:
>
> Hi All,
>
> Please find
and everything passed.
>
> Thanks Vyom.
>
> Kind regards,
> Patrick
>
> On 8 Jul 2020, at 06:49, Vyom Tiwari wrote:
>
> Hi Patrick,
> Thanks for testing, please find the latest webrev(
> http://cr.openjdk.java.net/~vtewari/8237858/webrev1.0/index.html). I
> fixed th
de is properly formatted and i am assuming that while pushing
there will not be any formatting issue.
Please let me know if I am missing something.
thanks,
Vyom
On Wed, Jul 15, 2020 at 12:44 AM Alan Bateman
wrote:
>
>
> On 14/07/2020 20:09, Daniel Fuchs wrote:
> > On 12/07/202
t;
> missing space just after else:
> 447 }else {
>
> best regards,
>
> -- daniel
>
> On 15/07/2020 09:14, Vyom Tiwari wrote:
> > Hi Alan,
> >
> > thanks for the review, I will definitely fix any formatting issue before
> > pushing the patch
looks ok to me.
Vyom
On Fri, Jul 24, 2020 at 6:05 AM Joe Darcy wrote:
> Hello,
>
> Please review the replacement of default constructors in various
> abstract classes in java.net with explicit constructors:
>
> webrev: http://cr.openjdk.java.net/~darcy/8250244.0/
> CSR: https://bugs.op
Hi Patrick,
Changes looks ok to me.
Thanks,
Vyom
On Wed, Jul 29, 2020 at 9:45 PM Patrick Concannon <
patrick.concan...@oracle.com> wrote:
> Hi,
>
> Could someone please review my fix for JDK-8246164 —
> ‘SendDatagramToBadAddress.java and ChangingAddress.java should be changed
> to explicitly requ
+1
Vyom
On Fri, Aug 28, 2020 at 8:24 PM Alexander Scherbatiy <
alexander.scherba...@bell-sw.com> wrote:
> On 28.08.2020 17:40, Alan Bateman wrote:
>
> > On 28/08/2020 15:32, Alexander Scherbatiy wrote:
> >>
> >> I run the java/nio/channels tests with the fix. There are five
> >> failed tests th
Hi,
I am facing the build issue with OpenJDK11(jdk11u). I am trying to build
jdk11u on Windows and I am getting the below error.
./src/java.base/windows/native/libnet/net_util_md.c(792): error C2065:
'TCP_INITIAL_RTO_NO_SYN_RETRANSMISSIONS': undeclared identifier
make[3]: **
51 matches
Mail list logo