Re: e Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

Hi Aleksei, Daniel, Great news, thanks a lot for all your help on cleaning up this patch! Cheers, - Anuraag On Wed, Jan 15, 2020, 01:14 Aleks Efimov wrote: > Thanks Daniel! > > Anuuraag, > I will update the copyrights, no need to send new patch. Then will > sponsor your change - commit and pu

Re: e Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

Thanks Daniel! Anuuraag, I will update the copyrights, no need to send new patch.  Then will sponsor your change - commit and push to jdk/jdk. Best Regards, Aleksei On 14/01/2020 11:06, Daniel Fuchs wrote: Hi Anuuraag, Aleksei, Looks good to me. Except that now copyright years must be up

Re: e Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

Hi Anuuraag, Aleksei, Looks good to me. Except that now copyright years must be updated to 2020. No need for a new webrev if that's the only change! best regards, -- daniel On 09/01/2020 17:17, Aleks Efimov wrote: Hi Anuuraag, Latest webrev: http://cr.openjdk.java.net/~aefimov/anuraaga/7006

e Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

Hi Anuuraag, Latest webrev: http://cr.openjdk.java.net/~aefimov/anuraaga/7006496/05/ Looks fine to me CI is also happy Best Regards, Aleksei On 09/01/2020 15:22, Anuraag Agrawal wrote: Hi all, Apologies for the spam. I made one more tweak to follow up the previous, and removed the default va

Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

Hi all, Apologies for the spam. I made one more tweak to follow up the previous, and removed the default value of -1 for lastRefresh. Now that there is no semantic meaning to -1, I think unspecified default (effectively zero) is more appropriate and easy to reason about. Inline patch follows dif

Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

Hi Daniel, Thanks for the review! On Thu, Jan 9, 2020 at 9:04 PM Daniel Fuchs wrote: > Hi, > > Two small remarks: > > ResolverConfigurationImpl.java: > >71 if (!l.contains(s)) { > > Maybe empty strings should be skipped here too. > Makes sense, added the check. > > I believe

Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

Hi, Two small remarks: ResolverConfigurationImpl.java: 71 if (!l.contains(s)) { Maybe empty strings should be skipped here too. I believe it would be more correct to change this line: 117 if (lastRefresh >= 0) { to 117 if (lastRefresh != -1) { as Sy

Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

I’m happy with the latest iteration of this code. It has my Review. -Chris. > On 9 Jan 2020, at 00:54, Aleks Efimov wrote: > > Got the testing results: the CI is happy with the last patch - no JNDI test > failures observed > > Kind Regards, > Aleksei > > On 08/01/2020 18:23, Aleks Efimov wro

Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

Got the testing results: the CI is happy with the last patch - no JNDI test failures observed Kind Regards, Aleksei On 08/01/2020 18:23, Aleks Efimov wrote: Hi Anuraag, I've uploaded your latest patch to the following location: http://cr.openjdk.java.net/~aefimov/anuraaga/7006496/04 The loca

Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

Hi Anuraag, I've uploaded your latest patch to the following location: http://cr.openjdk.java.net/~aefimov/anuraaga/7006496/04 The local Windows build and the acquired configuration are good. I will run you patch through our CI system and will update this thread once I get the results. Kind

Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

Hi Chris, Ah ok - I was worried it might be a backwards incompatible change, but I guess these are internal classes anyways. I have updated the patch, removing the exception handling logic so the errors propagate up. Inline patch follows diff --git a/src/java.base/windows/classes/sun/net/dns/Res

Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

> On 8 Jan 2020, at 05:12, Anuraag Agrawal wrote: > > Hi Chris, > > Happy new year - sorry for the long delay. I have made an updated patch > fixing the comment (missing comma), bumping MAX_STR_LEN, and tweaking the > exception behavior. > > Basically, as you pointed out loadDNSConfig itself

Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

Hi Chris, Happy new year - sorry for the long delay. I have made an updated patch fixing the comment (missing comma), bumping MAX_STR_LEN, and tweaking the exception behavior. Basically, as you pointed out loadDNSConfig itself does not throw any exception. The getAdapters function will throw a) O

Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

> On 27 Dec 2019, at 08:44, Anuraag Agrawal wrote: > > ... > On Tue, Dec 24, 2019 at 11:57 PM Chris Hegarty > wrote: > > ... > I think that this mainly looks good. A few small comments: > > 1) If getAdapters returns an error ( ret != ERROR_SUCCESS ), then the

Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

Hi Chris, Thanks for the comments! On Tue, Dec 24, 2019 at 11:57 PM Chris Hegarty wrote: > > > > On 24 Dec 2019, at 10:12, Aleks Efimov > wrote: > > > > Hi Anuraag, > > > > We need additional approval from openjdk reviewer. After that I will > sponsor your change. > > > > Merry Christmas and a

Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

> On 24 Dec 2019, at 10:12, Aleks Efimov wrote: > > Hi Anuraag, > > We need additional approval from openjdk reviewer. After that I will sponsor > your change. > > Merry Christmas and a Happy New Year, > Aleksei > > On 18/12/2019 06:08, Anuraag Agrawal wrote: >> ... >> http://cr.openjdk.ja

Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

Hi Aleksei, Thanks for the feedback. I had thought about it but felt that it might be better to leave as much business logic to Java vs native as possible as a general principle. But if it makes more sense to do it native here happy to make the change :) Thanks, - Anuraag On Thu, Dec 12, 2019, 0

Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

Hi Anuraag, The webrev with the latest patch can be viewed here: http://cr.openjdk.java.net/~aefimov/anuraaga/7006496/02 Thanks for replacing FirstDnsSuffix usages with DnsSuffix. I've double checked it with native application and can confirm that on my test host (Microsoft Windows Server 2016

Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

Hi Aleksei, Thanks for running the test. I had checked the global search list, but realize I didn't check connection-specific suffixes which were causing the issue you see. While the documentation states that FirstDnsSuffix is populated on Vista+, amazingly it doesn't seem to be https://docs.micr

Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

Hi Anuraag, The webrev with your latest changes can be found here:     http://cr.openjdk.java.net/~aefimov/anuraaga/7006496/01 The copyright changes looks good. The split by space also looks good with assumption that we will never get string with two spaces between IP addresses/DNS suffixes.

Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

Hi all, Thanks for the reviews. I've fixed the formatting errors, and I hope I fixed the copyright issue. I couldn't find documentation on the format of the copyright header, but assume the second number is the year of modification to update. I found ResolverConfigurationImpl.c to have an old one

Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

On 06/12/2019 15:34, Aleks Efimov wrote: : I've created webrev with your latest changes - it could be convenient for other reviewers: http://cr.openjdk.java.net/~aefimov/anuraaga/7006496/00 I will happily sponsor this change once it gets necessary approvals This could be split into two issues

Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

ts talking about „comma and space“ but the code below will only tokenize by space. Imguess the comment needs to be fixed, for the next review round. Gruss Bernd -- http://bernd.eckenfels.net *Von:*

Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

hing I noticed there are two comments talking about „comma and space“ but the code below will only tokenize by space. Imguess the comment needs to be fixed, for the next review round. Gruss Bernd -- http://bernd.eckenfels.net ---

Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

looks like a good idea (especially the additional cleanup for > nanos and arraylist) Just a minor thing I noticed there are two comments > talking about „comma and space“ but the code below will only tokenize by > space. Imguess the comment needs to be fixed, for the next review round. >

Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

review round. Gruss Bernd -- http://bernd.eckenfels.net Von: net-dev im Auftrag von Anuraag Agrawal Gesendet: Sonntag, November 24, 2019 10:47 AM An: net-dev@openjdk.java.net Betreff: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers Hello

[PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

Hello, While investigating DNS resolution failure using Netty ( https://github.com/netty/netty/issues/9796), I came across an old JDK bug where on Windows, the list of OS name servers includes those for disabled interfaces (https://bugs.openjdk.java.net/browse/JDK-7006496). This is problematic sin