date:20161221

Re: RFR 8168840: InetAddress.getByName() throws java.net.UnknownHostException no such interface when used with virtual interfaces on Solaris

2016-12-21 Thread Vyom Tewari


Hi All,

Please find the latest 
webrev(http://cr.openjdk.java.net/~vtewari/8168840/webrev0.1/index.html 
).


Thanks,

Vyom


On Tuesday 13 December 2016 01:38 PM, Langer, Christoph wrote:

Hi Vyom,

thanks for looking at this. Overall your fix and test look good to me.

The coding to do the parent search and if required a child search in 
Java_java_net_NetworkInterface_getByName0 could be done a bit more 
straightforward, e.g. like this:

 // search the list of interfaces by name
 // for virtual interfaces we need to find the parent first
 colonp = strchr(name_utf, ':');
 if (colonp == NULL) {
 searchName = name_utf;
 } else {
 jio_snprintf(pname, IFNAMESIZE, "%.*s", colonp - name_utf);
 searchName = pname;
 }
 curr = ifs;
 while (curr != NULL) {
 if (strcmp(searchName, curr->name) == 0) {
 break;
 }
 curr = curr->next;
 }

 // search the child list
 if (curr != NULL && colonp != NULL) {
 curr = curr->childs;
 while (curr != NULL) {
 if (strcmp(name_utf, curr->name) == 0) {
 break;
 }
 curr = curr->next;
 }
 }

Best regards
Christoph


-Original Message-
From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Vyom
Tewari
Sent: Dienstag, 13. Dezember 2016 04:47
To: net-dev 
Subject: RFR 8168840: InetAddress.getByName() throws
java.net.UnknownHostException no such interface when used with virtual
interfaces on Solaris

Hi,

Please review the code changes for below issue.

BugId: https://bugs.openjdk.java.net/browse/JDK-8168840

webrev :
http://cr.openjdk.java.net/~vtewari/8168840/webrev0.0/index.html


Thanks,

Vyom

RE: RFR 8168840: InetAddress.getByName() throws java.net.UnknownHostException no such interface when used with virtual interfaces on Solaris

2016-12-21 Thread Langer, Christoph

Hi Vyom,

looks good, thanks for the update.

Minor formatting:
- Add a blank line between line 258/259 and 268/269 in the new file version.
- line 259 //search the child list - add a space between "//" and 
"search..."

Disclaimer: I'm not an official reviewer.

Best regards
Christoph

> -Original Message-
> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Vyom
> Tewari
> Sent: Mittwoch, 21. Dezember 2016 09:51
> Cc: net-dev 
> Subject: Re: RFR 8168840: InetAddress.getByName() throws
> java.net.UnknownHostException no such interface when used with virtual
> interfaces on Solaris
> 
> Hi All,
> 
> Please find the latest
> webrev(http://cr.openjdk.java.net/~vtewari/8168840/webrev0.1/index.html
> ).
> 
> Thanks,
> 
> Vyom
> 
> 
> On Tuesday 13 December 2016 01:38 PM, Langer, Christoph wrote:
> > Hi Vyom,
> >
> > thanks for looking at this. Overall your fix and test look good to me.
> >
> > The coding to do the parent search and if required a child search in
> Java_java_net_NetworkInterface_getByName0 could be done a bit more
> straightforward, e.g. like this:
> >
> >  // search the list of interfaces by name
> >  // for virtual interfaces we need to find the parent first
> >  colonp = strchr(name_utf, ':');
> >  if (colonp == NULL) {
> >  searchName = name_utf;
> >  } else {
> >  jio_snprintf(pname, IFNAMESIZE, "%.*s", colonp - name_utf);
> >  searchName = pname;
> >  }
> >  curr = ifs;
> >  while (curr != NULL) {
> >  if (strcmp(searchName, curr->name) == 0) {
> >  break;
> >  }
> >  curr = curr->next;
> >  }
> >
> >  // search the child list
> >  if (curr != NULL && colonp != NULL) {
> >  curr = curr->childs;
> >  while (curr != NULL) {
> >  if (strcmp(name_utf, curr->name) == 0) {
> >  break;
> >  }
> >  curr = curr->next;
> >  }
> >  }
> >
> > Best regards
> > Christoph
> >
> >> -Original Message-
> >> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> Vyom
> >> Tewari
> >> Sent: Dienstag, 13. Dezember 2016 04:47
> >> To: net-dev 
> >> Subject: RFR 8168840: InetAddress.getByName() throws
> >> java.net.UnknownHostException no such interface when used with virtual
> >> interfaces on Solaris
> >>
> >> Hi,
> >>
> >> Please review the code changes for below issue.
> >>
> >> BugId: https://bugs.openjdk.java.net/browse/JDK-8168840
> >>
> >> webrev :
> >> http://cr.openjdk.java.net/~vtewari/8168840/webrev0.0/index.html
> >> 
> >>
> >> Thanks,
> >>
> >> Vyom

Re: RFR 8168840: InetAddress.getByName() throws java.net.UnknownHostException no such interface when used with virtual interfaces on Solaris

2016-12-21 Thread Vyom Tewari

incorporated the 
comments(http://cr.openjdk.java.net/~vtewari/8168840/webrev0.2/index.html 
).


Thanks,

Vyom


On Wednesday 21 December 2016 02:58 PM, Langer, Christoph wrote:

Hi Vyom,

looks good, thanks for the update.

Minor formatting:
- Add a blank line between line 258/259 and 268/269 in the new file version.
- line 259 //search the child list - add a space between "//" and 
"search..."

Disclaimer: I'm not an official reviewer.

Best regards
Christoph


-Original Message-
From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Vyom
Tewari
Sent: Mittwoch, 21. Dezember 2016 09:51
Cc: net-dev 
Subject: Re: RFR 8168840: InetAddress.getByName() throws
java.net.UnknownHostException no such interface when used with virtual
interfaces on Solaris

Hi All,

Please find the latest
webrev(http://cr.openjdk.java.net/~vtewari/8168840/webrev0.1/index.html
).

Thanks,

Vyom


On Tuesday 13 December 2016 01:38 PM, Langer, Christoph wrote:

Hi Vyom,

thanks for looking at this. Overall your fix and test look good to me.

The coding to do the parent search and if required a child search in

Java_java_net_NetworkInterface_getByName0 could be done a bit more
straightforward, e.g. like this:

  // search the list of interfaces by name
  // for virtual interfaces we need to find the parent first
  colonp = strchr(name_utf, ':');
  if (colonp == NULL) {
  searchName = name_utf;
  } else {
  jio_snprintf(pname, IFNAMESIZE, "%.*s", colonp - name_utf);
  searchName = pname;
  }
  curr = ifs;
  while (curr != NULL) {
  if (strcmp(searchName, curr->name) == 0) {
  break;
  }
  curr = curr->next;
  }

  // search the child list
  if (curr != NULL && colonp != NULL) {
  curr = curr->childs;
  while (curr != NULL) {
  if (strcmp(name_utf, curr->name) == 0) {
  break;
  }
  curr = curr->next;
  }
  }

Best regards
Christoph


-Original Message-
From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of

Vyom

Tewari
Sent: Dienstag, 13. Dezember 2016 04:47
To: net-dev 
Subject: RFR 8168840: InetAddress.getByName() throws
java.net.UnknownHostException no such interface when used with virtual
interfaces on Solaris

Hi,

Please review the code changes for below issue.

BugId: https://bugs.openjdk.java.net/browse/JDK-8168840

webrev :
http://cr.openjdk.java.net/~vtewari/8168840/webrev0.0/index.html


Thanks,

Vyom

Re: RFR 8168840: InetAddress.getByName() throws java.net.UnknownHostException no such interface when used with virtual interfaces on Solaris

2016-12-21 Thread Chris Hegarty


Hi Vyom,

Sorry, I'm missing the connection between Inet6Address and
NetworkInterface.getByName, how do these interact?

-Chris.

On 21/12/16 10:20, Vyom Tewari wrote:

incorporated the
comments(http://cr.openjdk.java.net/~vtewari/8168840/webrev0.2/index.html 
).


Thanks,

Vyom


On Wednesday 21 December 2016 02:58 PM, Langer, Christoph wrote:

Hi Vyom,

looks good, thanks for the update.

Minor formatting:
- Add a blank line between line 258/259 and 268/269 in the new file
version.
- line 259 //search the child list - add a space between "//" and
"search..."

Disclaimer: I'm not an official reviewer.

Best regards
Christoph


-Original Message-
From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
Vyom
Tewari
Sent: Mittwoch, 21. Dezember 2016 09:51
Cc: net-dev 
Subject: Re: RFR 8168840: InetAddress.getByName() throws
java.net.UnknownHostException no such interface when used with virtual
interfaces on Solaris

Hi All,

Please find the latest
webrev(http://cr.openjdk.java.net/~vtewari/8168840/webrev0.1/index.html
).

Thanks,

Vyom


On Tuesday 13 December 2016 01:38 PM, Langer, Christoph wrote:

Hi Vyom,

thanks for looking at this. Overall your fix and test look good to me.

The coding to do the parent search and if required a child search in

Java_java_net_NetworkInterface_getByName0 could be done a bit more
straightforward, e.g. like this:

  // search the list of interfaces by name
  // for virtual interfaces we need to find the parent first
  colonp = strchr(name_utf, ':');
  if (colonp == NULL) {
  searchName = name_utf;
  } else {
  jio_snprintf(pname, IFNAMESIZE, "%.*s", colonp - name_utf);
  searchName = pname;
  }
  curr = ifs;
  while (curr != NULL) {
  if (strcmp(searchName, curr->name) == 0) {
  break;
  }
  curr = curr->next;
  }

  // search the child list
  if (curr != NULL && colonp != NULL) {
  curr = curr->childs;
  while (curr != NULL) {
  if (strcmp(name_utf, curr->name) == 0) {
  break;
  }
  curr = curr->next;
  }
  }

Best regards
Christoph


-Original Message-
From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of

Vyom

Tewari
Sent: Dienstag, 13. Dezember 2016 04:47
To: net-dev 
Subject: RFR 8168840: InetAddress.getByName() throws
java.net.UnknownHostException no such interface when used with virtual
interfaces on Solaris

Hi,

Please review the code changes for below issue.

BugId: https://bugs.openjdk.java.net/browse/JDK-8168840

webrev :
http://cr.openjdk.java.net/~vtewari/8168840/webrev0.0/index.html


Thanks,

Vyom

RE: RFR:8170868: DefaultProxySelector should use system defaults on Windows, MacOS and Gnome

2016-12-21 Thread Zeller, Arno

Hi Volker,

thanks for your valuable comments! I have a new patch ready that should address 
your issues and contains also a forgotten change to the map file...

New webrev: http://cr.openjdk.java.net/~clanger/webrevs/8170868.1/


>- make/lib/NetworkingLibraries.gmk
...
>Have you tried to use
>LIBNET_EXCLUDE_FILES :=
>$(JDK_TOPDIR)/src/java.base/unix/native/libnet/DefaultProxySelector.c
>
>I think this should work and it would mke it possible to rename:
>src/java.base/macosx/native/libnet/DefaultProxySelector_mac.c
>to:
>src/java.base/macosx/native/libnet/DefaultProxySelector.c
>which is much nicer IMHO :)

Great idea - it works and is of course the much nicer solution! 

>- DefaultProxySelector.java
>
>322 return proxyList == null ? Arrays.asList(Proxy.NO_PROXY) :
>proxyList;
>
>Not sure if it would make sense to preallocate a static List with a single
>Proxy.NO_PROXY element and always return that if proxyList equals null?

I return a new List object each time, because the select(URI uri) method does 
not state anything about
not modifying the returned list. In case I return a static list containing only 
the NO_PROXY element
a caller could remove the object from the list and other caller would use the 
same modified list.
To avoid this I always create a new List object.

>- java.base/unix/native/libnet/DefaultProxySelector.c
>
>You've removed "#include ". Have you built on all Unix platforms
>(AIX, Solaris) to make sure you don't break anything?

It compiled on Linux, AIX, Solaris and Mac without problems for me.

>- java.base/windows/native/libnet/DefaultProxySelector.c
>
>Not sure if I understand this right, but in the gconf case above you insert all
>proxies returned by "g_proxy_resolver_lookup" into the prox-list returned by
>DefaultProxySelector_getSystemProxies. In the Windows case you write:
>
> 247* From MSDN: The proxy server list contains one or more of
>the following strings separated by semicolons or whitespace.
> 248* ([=]["://"][":"])
> 249* We will only take the first entry here because the
>java.net.Proxy class has only one entry.
>
>Why can't you build up a proxy list here in the same way you do it for the
>gconf case on Unix?

Sorry - I just forgot to implement it. Good that you found it. The new webrev 
contains the missing functionality.

>- src/java.base/macosx/native/libnet/DefaultProxySelector_mac.c
>
>  76 #define kResolveProxyRunLoopMode
>CFSTR("com.sap.jvm.DefaultProxySelector")
>
>
>I'm not familiar with the Mac programming model, but I don't think
>"com.sap.jvm.DefaultProxySelector" is a good name for
>kResolveProxyRunLoopMode. It should be something like
>"java.net.DefaultProxySelector" but I'm open for better proposals :)

You are right - I changed it to "sun.net.spi.DefaultProxySelector".

>PS: for your next RFR, can you please also add the estimated sisze and the bug
>id to the subject line (e.g. "RFR(M):
>8170868:DefaultProxySelector should..."). This makes it much easier to find a
>review thread :)

I'll do my best next time...

Best regards,
Arno

>On Wed, Dec 14, 2016 at 5:06 PM, Zeller, Arno  wrote:
>> Hi,
>>
>> can you please review my proposal for bug 8170868 - DefaultProxySelector
>should use system defaults on Windows, MacOS and Gnome.
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8170868
>>
>> Webrev:
>> http://cr.openjdk.java.net/~clanger/webrevs/8170868.0/
>>
>> Thanks a lot,
>> Arno Zeller
>>

RE: RFR:8170868: DefaultProxySelector should use system defaults on Windows, MacOS and Gnome

2016-12-21 Thread Langer, Christoph

Hi Arno,

this looks good. I had already reviewed your change.

Just as for one thing that Volker mentioned:

>- DefaultProxySelector.java
>
>322 return proxyList == null ? Arrays.asList(Proxy.NO_PROXY) : 
>proxyList;
>
>Not sure if it would make sense to preallocate a static List with a single
>Proxy.NO_PROXY element and always return that if proxyList equals null?

I also have the concern that if you return one static instance of the List, 
then someone could modify it. Alternatively one could preallocate a static list 
and return Collections.unmodifiableList() from it. But I don't know if that 
makes things faster or is even more overhead compared to always generating a 
new List.

Best regards
Christoph

> -Original Message-
> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Zeller,
> Arno
> Sent: Mittwoch, 21. Dezember 2016 13:53
> To: Volker Simonis 
> Cc: net-dev@openjdk.java.net
> Subject: RE: RFR:8170868: DefaultProxySelector should use system defaults on
> Windows, MacOS and Gnome
> 
> Hi Volker,
> 
> thanks for your valuable comments! I have a new patch ready that should
> address your issues and contains also a forgotten change to the map file...
> 
> New webrev: http://cr.openjdk.java.net/~clanger/webrevs/8170868.1/
> 
> 
> >- make/lib/NetworkingLibraries.gmk
> ...
> >Have you tried to use
> >LIBNET_EXCLUDE_FILES :=
> >$(JDK_TOPDIR)/src/java.base/unix/native/libnet/DefaultProxySelector.c
> >
> >I think this should work and it would mke it possible to rename:
> >src/java.base/macosx/native/libnet/DefaultProxySelector_mac.c
> >to:
> >src/java.base/macosx/native/libnet/DefaultProxySelector.c
> >which is much nicer IMHO :)
> 
> Great idea - it works and is of course the much nicer solution!
> 
> >- DefaultProxySelector.java
> >
> >322 return proxyList == null ? Arrays.asList(Proxy.NO_PROXY) :
> >proxyList;
> >
> >Not sure if it would make sense to preallocate a static List with a single
> >Proxy.NO_PROXY element and always return that if proxyList equals null?
> 
> I return a new List object each time, because the select(URI uri) method does
> not state anything about
> not modifying the returned list. In case I return a static list containing 
> only the
> NO_PROXY element
> a caller could remove the object from the list and other caller would use the
> same modified list.
> To avoid this I always create a new List object.
> 
> >- java.base/unix/native/libnet/DefaultProxySelector.c
> >
> >You've removed "#include ". Have you built on all Unix platforms
> >(AIX, Solaris) to make sure you don't break anything?
> 
> It compiled on Linux, AIX, Solaris and Mac without problems for me.
> 
> >- java.base/windows/native/libnet/DefaultProxySelector.c
> >
> >Not sure if I understand this right, but in the gconf case above you insert 
> >all
> >proxies returned by "g_proxy_resolver_lookup" into the prox-list returned by
> >DefaultProxySelector_getSystemProxies. In the Windows case you write:
> >
> > 247* From MSDN: The proxy server list contains one or more of
> >the following strings separated by semicolons or whitespace.
> > 248* ([=]["://"][":"])
> > 249* We will only take the first entry here because the
> >java.net.Proxy class has only one entry.
> >
> >Why can't you build up a proxy list here in the same way you do it for the
> >gconf case on Unix?
> 
> Sorry - I just forgot to implement it. Good that you found it. The new webrev
> contains the missing functionality.
> 
> >- src/java.base/macosx/native/libnet/DefaultProxySelector_mac.c
> >
> >  76 #define kResolveProxyRunLoopMode
> >CFSTR("com.sap.jvm.DefaultProxySelector")
> >
> >
> >I'm not familiar with the Mac programming model, but I don't think
> >"com.sap.jvm.DefaultProxySelector" is a good name for
> >kResolveProxyRunLoopMode. It should be something like
> >"java.net.DefaultProxySelector" but I'm open for better proposals :)
> 
> You are right - I changed it to "sun.net.spi.DefaultProxySelector".
> 
> >PS: for your next RFR, can you please also add the estimated sisze and the 
> >bug
> >id to the subject line (e.g. "RFR(M):
> >8170868:DefaultProxySelector should..."). This makes it much easier to find a
> >review thread :)
> 
> I'll do my best next time...
> 
> Best regards,
> Arno
> 
> >On Wed, Dec 14, 2016 at 5:06 PM, Zeller, Arno  wrote:
> >> Hi,
> >>
> >> can you please review my proposal for bug 8170868 - DefaultProxySelector
> >should use system defaults on Windows, MacOS and Gnome.
> >>
> >> Bug:
> >> https://bugs.openjdk.java.net/browse/JDK-8170868
> >>
> >> Webrev:
> >> http://cr.openjdk.java.net/~clanger/webrevs/8170868.0/
> >>
> >> Thanks a lot,
> >> Arno Zeller
> >>

Re: RFR 8168840: InetAddress.getByName() throws java.net.UnknownHostException no such interface when used with virtual interfaces on Solaris

2016-12-21 Thread Vyom Tewari


Hi Chris,

If you create the Inet6Address  using the following constructor 
"Inet6Address (String , byte[], String )" then it will call the 
following private method " initstr (hostName, addr, ifname);" and it you 
see the implementation of this method then you will see the below code.


###

try {
NetworkInterface nif = NetworkInterface.getByName (ifname);
if (nif == null) {
throw new UnknownHostException ("no such interface " + 
ifname);

}
initif (hostName, addr, nif);




So this is the connection between Inet6Address and 
NetworkInterface.getByName but same is not the case  for ipv4 address. 
As we added the scope name recently(not very recent but after 
jdk1.7.0_79) so if you pass the virtual sub interface interface 
like(2001:7a8:b0cd:1:0:0:0:17%net0:1) then it will fail to get the sub 
interface and UHE will be thrown.


to handle this i did the code change.


Thanks,
Vyom

On 12/21/2016 5:04 PM, Chris Hegarty wrote:

Hi Vyom,

Sorry, I'm missing the connection between Inet6Address and
NetworkInterface.getByName, how do these interact?

-Chris.

On 21/12/16 10:20, Vyom Tewari wrote:

incorporated the
comments(http://cr.openjdk.java.net/~vtewari/8168840/webrev0.2/index.html 
).



Thanks,

Vyom


On Wednesday 21 December 2016 02:58 PM, Langer, Christoph wrote:

Hi Vyom,

looks good, thanks for the update.

Minor formatting:
- Add a blank line between line 258/259 and 268/269 in the new file
version.
- line 259 //search the child list - add a space between "//" and
"search..."

Disclaimer: I'm not an official reviewer.

Best regards
Christoph


-Original Message-
From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
Vyom
Tewari
Sent: Mittwoch, 21. Dezember 2016 09:51
Cc: net-dev 
Subject: Re: RFR 8168840: InetAddress.getByName() throws
java.net.UnknownHostException no such interface when used with virtual
interfaces on Solaris

Hi All,

Please find the latest
webrev(http://cr.openjdk.java.net/~vtewari/8168840/webrev0.1/index.html 


).

Thanks,

Vyom


On Tuesday 13 December 2016 01:38 PM, Langer, Christoph wrote:

Hi Vyom,

thanks for looking at this. Overall your fix and test look good to 
me.


The coding to do the parent search and if required a child search in

Java_java_net_NetworkInterface_getByName0 could be done a bit more
straightforward, e.g. like this:

  // search the list of interfaces by name
  // for virtual interfaces we need to find the parent first
  colonp = strchr(name_utf, ':');
  if (colonp == NULL) {
  searchName = name_utf;
  } else {
  jio_snprintf(pname, IFNAMESIZE, "%.*s", colonp - name_utf);
  searchName = pname;
  }
  curr = ifs;
  while (curr != NULL) {
  if (strcmp(searchName, curr->name) == 0) {
  break;
  }
  curr = curr->next;
  }

  // search the child list
  if (curr != NULL && colonp != NULL) {
  curr = curr->childs;
  while (curr != NULL) {
  if (strcmp(name_utf, curr->name) == 0) {
  break;
  }
  curr = curr->next;
  }
  }

Best regards
Christoph


-Original Message-
From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of

Vyom

Tewari
Sent: Dienstag, 13. Dezember 2016 04:47
To: net-dev 
Subject: RFR 8168840: InetAddress.getByName() throws
java.net.UnknownHostException no such interface when used with 
virtual

interfaces on Solaris

Hi,

Please review the code changes for below issue.

BugId: https://bugs.openjdk.java.net/browse/JDK-8168840

webrev :
http://cr.openjdk.java.net/~vtewari/8168840/webrev0.0/index.html


Thanks,

Vyom

Re: RFR 8168840: InetAddress.getByName() throws java.net.UnknownHostException no such interface when used with virtual interfaces on Solaris

2016-12-21 Thread Chris Hegarty


Vyom,

Thank you for the explanation. Makes sense.

Your changes look fine as are, but maybe '250 *colonP = '\0';'
would be clearer ( though I do note that we do *name_colonP = 0;
elsewhere in this file ).

-Chris.

On 21/12/16 13:41, Vyom Tewari wrote:

Hi Chris,

If you create the Inet6Address  using the following constructor
"Inet6Address (String , byte[], String )" then it will call the
following private method " initstr (hostName, addr, ifname);" and it you
see the implementation of this method then you will see the below code.

###

try {
NetworkInterface nif = NetworkInterface.getByName (ifname);
if (nif == null) {
throw new UnknownHostException ("no such interface " +
ifname);
}
initif (hostName, addr, nif);




So this is the connection between Inet6Address and
NetworkInterface.getByName but same is not the case  for ipv4 address.
As we added the scope name recently(not very recent but after
jdk1.7.0_79) so if you pass the virtual sub interface interface
like(2001:7a8:b0cd:1:0:0:0:17%net0:1) then it will fail to get the sub
interface and UHE will be thrown.

to handle this i did the code change.


Thanks,
Vyom

On 12/21/2016 5:04 PM, Chris Hegarty wrote:

Hi Vyom,

Sorry, I'm missing the connection between Inet6Address and
NetworkInterface.getByName, how do these interact?

-Chris.

On 21/12/16 10:20, Vyom Tewari wrote:

incorporated the
comments(http://cr.openjdk.java.net/~vtewari/8168840/webrev0.2/index.html
).


Thanks,

Vyom


On Wednesday 21 December 2016 02:58 PM, Langer, Christoph wrote:

Hi Vyom,

looks good, thanks for the update.

Minor formatting:
- Add a blank line between line 258/259 and 268/269 in the new file
version.
- line 259 //search the child list - add a space between "//" and
"search..."

Disclaimer: I'm not an official reviewer.

Best regards
Christoph


-Original Message-
From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
Vyom
Tewari
Sent: Mittwoch, 21. Dezember 2016 09:51
Cc: net-dev 
Subject: Re: RFR 8168840: InetAddress.getByName() throws
java.net.UnknownHostException no such interface when used with virtual
interfaces on Solaris

Hi All,

Please find the latest
webrev(http://cr.openjdk.java.net/~vtewari/8168840/webrev0.1/index.html

).

Thanks,

Vyom


On Tuesday 13 December 2016 01:38 PM, Langer, Christoph wrote:

Hi Vyom,

thanks for looking at this. Overall your fix and test look good to
me.

The coding to do the parent search and if required a child search in

Java_java_net_NetworkInterface_getByName0 could be done a bit more
straightforward, e.g. like this:

  // search the list of interfaces by name
  // for virtual interfaces we need to find the parent first
  colonp = strchr(name_utf, ':');
  if (colonp == NULL) {
  searchName = name_utf;
  } else {
  jio_snprintf(pname, IFNAMESIZE, "%.*s", colonp - name_utf);
  searchName = pname;
  }
  curr = ifs;
  while (curr != NULL) {
  if (strcmp(searchName, curr->name) == 0) {
  break;
  }
  curr = curr->next;
  }

  // search the child list
  if (curr != NULL && colonp != NULL) {
  curr = curr->childs;
  while (curr != NULL) {
  if (strcmp(name_utf, curr->name) == 0) {
  break;
  }
  curr = curr->next;
  }
  }

Best regards
Christoph


-Original Message-
From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of

Vyom

Tewari
Sent: Dienstag, 13. Dezember 2016 04:47
To: net-dev 
Subject: RFR 8168840: InetAddress.getByName() throws
java.net.UnknownHostException no such interface when used with
virtual
interfaces on Solaris

Hi,

Please review the code changes for below issue.

BugId: https://bugs.openjdk.java.net/browse/JDK-8168840

webrev :
http://cr.openjdk.java.net/~vtewari/8168840/webrev0.0/index.html


Thanks,

Vyom

Re: RFR 8168840: InetAddress.getByName() throws java.net.UnknownHostException no such interface when used with virtual interfaces on Solaris

2016-12-21 Thread Vyom Tewari


Hi Chris,

i will do the change( *colonP = '\0';') before pushing still i have to 
run the jprt. Once it is done and clean i will do the suggested change 
and push the code.


thanks,

Vyom


On 12/21/2016 7:23 PM, Chris Hegarty wrote:

Vyom,

Thank you for the explanation. Makes sense.

Your changes look fine as are, but maybe '250 *colonP = '\0';'
would be clearer ( though I do note that we do *name_colonP = 0;
elsewhere in this file ).

-Chris.

On 21/12/16 13:41, Vyom Tewari wrote:

Hi Chris,

If you create the Inet6Address  using the following constructor
"Inet6Address (String , byte[], String )" then it will call the
following private method " initstr (hostName, addr, ifname);" and it you
see the implementation of this method then you will see the below code.

###

try {
NetworkInterface nif = NetworkInterface.getByName (ifname);
if (nif == null) {
throw new UnknownHostException ("no such interface " +
ifname);
}
initif (hostName, addr, nif);




So this is the connection between Inet6Address and
NetworkInterface.getByName but same is not the case  for ipv4 address.
As we added the scope name recently(not very recent but after
jdk1.7.0_79) so if you pass the virtual sub interface interface
like(2001:7a8:b0cd:1:0:0:0:17%net0:1) then it will fail to get the sub
interface and UHE will be thrown.

to handle this i did the code change.


Thanks,
Vyom

On 12/21/2016 5:04 PM, Chris Hegarty wrote:

Hi Vyom,

Sorry, I'm missing the connection between Inet6Address and
NetworkInterface.getByName, how do these interact?

-Chris.

On 21/12/16 10:20, Vyom Tewari wrote:

incorporated the
comments(http://cr.openjdk.java.net/~vtewari/8168840/webrev0.2/index.html 


).


Thanks,

Vyom


On Wednesday 21 December 2016 02:58 PM, Langer, Christoph wrote:

Hi Vyom,

looks good, thanks for the update.

Minor formatting:
- Add a blank line between line 258/259 and 268/269 in the new file
version.
- line 259 //search the child list - add a space between "//" and
"search..."

Disclaimer: I'm not an official reviewer.

Best regards
Christoph


-Original Message-
From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
Vyom
Tewari
Sent: Mittwoch, 21. Dezember 2016 09:51
Cc: net-dev 
Subject: Re: RFR 8168840: InetAddress.getByName() throws
java.net.UnknownHostException no such interface when used with 
virtual

interfaces on Solaris

Hi All,

Please find the latest
webrev(http://cr.openjdk.java.net/~vtewari/8168840/webrev0.1/index.html 



). 



Thanks,

Vyom


On Tuesday 13 December 2016 01:38 PM, Langer, Christoph wrote:

Hi Vyom,

thanks for looking at this. Overall your fix and test look good to
me.

The coding to do the parent search and if required a child 
search in

Java_java_net_NetworkInterface_getByName0 could be done a bit more
straightforward, e.g. like this:

  // search the list of interfaces by name
  // for virtual interfaces we need to find the parent first
  colonp = strchr(name_utf, ':');
  if (colonp == NULL) {
  searchName = name_utf;
  } else {
  jio_snprintf(pname, IFNAMESIZE, "%.*s", colonp - 
name_utf);

  searchName = pname;
  }
  curr = ifs;
  while (curr != NULL) {
  if (strcmp(searchName, curr->name) == 0) {
  break;
  }
  curr = curr->next;
  }

  // search the child list
  if (curr != NULL && colonp != NULL) {
  curr = curr->childs;
  while (curr != NULL) {
  if (strcmp(name_utf, curr->name) == 0) {
  break;
  }
  curr = curr->next;
  }
  }

Best regards
Christoph


-Original Message-
From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On 
Behalf Of

Vyom

Tewari
Sent: Dienstag, 13. Dezember 2016 04:47
To: net-dev 
Subject: RFR 8168840: InetAddress.getByName() throws
java.net.UnknownHostException no such interface when used with
virtual
interfaces on Solaris

Hi,

Please review the code changes for below issue.

BugId: https://bugs.openjdk.java.net/browse/JDK-8168840

webrev :
http://cr.openjdk.java.net/~vtewari/8168840/webrev0.0/index.html
 



Thanks,

Vyom

RFR(M): 8167420: Fixes for InetAddressImpl native coding on Linux/Unix platforms

2016-12-21 Thread Langer, Christoph

Hi,

after pushing the change for 8171075 
(http://hg.openjdk.java.net/jdk9/dev/jdk/rev/3edecfdfe4de), I have updated my 
patch for 8167420 and I have also updated the bug description to reflect the 
reduced scope.

Bug: https://bugs.openjdk.java.net/browse/JDK-8167420
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8167420.0/

The change now merely is code cleanup which should not change the current 
behavior (apart from the Solaris isspace(hostname[0]) check, which I think is 
not needed at least on the Solaris versions I worked with).

>From my point of view this change is ready to push and it runs nightly in our 
>builds/tests of OpenJDK on several platforms. Though I guess you want to 
>postpone this to JDK10, I'd be glad if you could still consider it for JDK9. 
>But I won't pressure otherwise ;-)

Thanks & Best regards
Christoph

RFR(M): 8167457: Fixes for InetAddressImpl native coding on Windows

2016-12-21 Thread Langer, Christoph

Hi again,

after pushing the change for 8171077 
(http://hg.openjdk.java.net/jdk9/dev/jdk/rev/24f8703890b2), I have updated my 
patch for 8167457 and I have also updated the bug description to reflect the 
reduced scope.

Bug: https://bugs.openjdk.java.net/browse/JDK-8167457
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8167457.0/

The change now merely is code cleanup which should not change the current 
behavior apart from removing the isspace(hostname[0]) check and the 
isDottedIPAddress check in Java_java_net_Inet4AddressImpl_lookupAllHostAddr. 
But testing lets me think it is not needed any longer for the supported Windows 
versions of JDK9.

Same as with 8167420, from my point of view this change is ready to push and it 
runs nightly in our builds/tests of OpenJDK on several platforms. Though I 
guess you want to postpone this to JDK10, I'd be glad if you could still 
consider it for JDK9. But I won't pressure otherwise ;-)

Thanks & Best regards
Christoph

Re: RFR:8170868: DefaultProxySelector should use system defaults on Windows, MacOS and Gnome

2016-12-21 Thread Vyom Tewari


Hi Arno,

I suggest you to call "CHECK_NULL_RETURN" after below line in 
DefaultProxySelector.c
// create an empty LinkedList to add the Proxy objects.
+ proxyList = (*env)->NewObject(env, list_class, list_ctrID); in 
windows/native/libnet/DefaultProxySelector.c file you remove the couple of 
"CHECK_NULL_RETURN"
 
 jstring jhost;

- if (pport == 0)
- pport = defport;
- jhost = (*env)->NewStringUTF(env, phost);
- CHECK_NULL_RETURN(jhost, NULL);
- isa = (*env)->CallStaticObjectMethod(env, isaddr_class, 
isaddr_createUnresolvedID, jhost, pport);

- CHECK_NULL_RETURN(isa, NULL);
- proxy = (*env)->NewObject(env, proxy_class, proxy_ctrID, type_proxy, isa);
- return proxy;
+ if (portVal == 0)
+ portVal = defport;
+ jhost = (*env)->NewString(env, phost, (jsize)wcslen(phost));
+ if (jhost != NULL) {
+ isa = (*env)->CallStaticObjectMethod(env, isaddr_class, 
isaddr_createUnresolvedID, jhost, portVal);

+ if (isa != NULL) {
+ jobject proxy = (*env)->NewObject(env, proxy_class, proxy_ctrID, 
type_proxy, isa);

+ if (proxy != NULL) {
+ (*env)->CallBooleanMethod(env, proxy_list, list_addID, proxy);
+ }
+ }
+ }
 }


Is there is specific reason behind removing these checks ?

Thanks,
Vyom


On 12/21/2016 6:23 PM, Zeller, Arno wrote:

Hi Volker,

thanks for your valuable comments! I have a new patch ready that should address 
your issues and contains also a forgotten change to the map file...

New webrev: http://cr.openjdk.java.net/~clanger/webrevs/8170868.1/



- make/lib/NetworkingLibraries.gmk

...

Have you tried to use
LIBNET_EXCLUDE_FILES :=
$(JDK_TOPDIR)/src/java.base/unix/native/libnet/DefaultProxySelector.c

I think this should work and it would mke it possible to rename:
src/java.base/macosx/native/libnet/DefaultProxySelector_mac.c
to:
src/java.base/macosx/native/libnet/DefaultProxySelector.c
which is much nicer IMHO :)

Great idea - it works and is of course the much nicer solution!


- DefaultProxySelector.java

322 return proxyList == null ? Arrays.asList(Proxy.NO_PROXY) :
proxyList;

Not sure if it would make sense to preallocate a static List with a single
Proxy.NO_PROXY element and always return that if proxyList equals null?

I return a new List object each time, because the select(URI uri) method does 
not state anything about
not modifying the returned list. In case I return a static list containing only 
the NO_PROXY element
a caller could remove the object from the list and other caller would use the 
same modified list.
To avoid this I always create a new List object.


- java.base/unix/native/libnet/DefaultProxySelector.c

You've removed "#include ". Have you built on all Unix platforms
(AIX, Solaris) to make sure you don't break anything?

It compiled on Linux, AIX, Solaris and Mac without problems for me.


- java.base/windows/native/libnet/DefaultProxySelector.c

Not sure if I understand this right, but in the gconf case above you insert all
proxies returned by "g_proxy_resolver_lookup" into the prox-list returned by
DefaultProxySelector_getSystemProxies. In the Windows case you write:

247* From MSDN: The proxy server list contains one or more of
the following strings separated by semicolons or whitespace.
248* ([=]["://"][":"])
249* We will only take the first entry here because the
java.net.Proxy class has only one entry.

Why can't you build up a proxy list here in the same way you do it for the
gconf case on Unix?

Sorry - I just forgot to implement it. Good that you found it. The new webrev 
contains the missing functionality.


- src/java.base/macosx/native/libnet/DefaultProxySelector_mac.c

  76 #define kResolveProxyRunLoopMode
CFSTR("com.sap.jvm.DefaultProxySelector")


I'm not familiar with the Mac programming model, but I don't think
"com.sap.jvm.DefaultProxySelector" is a good name for
kResolveProxyRunLoopMode. It should be something like
"java.net.DefaultProxySelector" but I'm open for better proposals :)

You are right - I changed it to "sun.net.spi.DefaultProxySelector".


PS: for your next RFR, can you please also add the estimated sisze and the bug
id to the subject line (e.g. "RFR(M):
8170868:DefaultProxySelector should..."). This makes it much easier to find a
review thread :)

I'll do my best next time...

Best regards,
Arno


On Wed, Dec 14, 2016 at 5:06 PM, Zeller, Arno  wrote:

Hi,

can you please review my proposal for bug 8170868 - DefaultProxySelector

should use system defaults on Windows, MacOS and Gnome.

Bug:
https://bugs.openjdk.java.net/browse/JDK-8170868

Webrev:
http://cr.openjdk.java.net/~clanger/webrevs/8170868.0/

Thanks a lot,
Arno Zeller

Re: RFR 8168840: InetAddress.getByName() throws java.net.UnknownHostException no such interface when used with virtual interfaces on Solaris

RE: RFR 8168840: InetAddress.getByName() throws java.net.UnknownHostException no such interface when used with virtual interfaces on Solaris

Re: RFR 8168840: InetAddress.getByName() throws java.net.UnknownHostException no such interface when used with virtual interfaces on Solaris

Re: RFR 8168840: InetAddress.getByName() throws java.net.UnknownHostException no such interface when used with virtual interfaces on Solaris

RE: RFR:8170868: DefaultProxySelector should use system defaults on Windows, MacOS and Gnome

RE: RFR:8170868: DefaultProxySelector should use system defaults on Windows, MacOS and Gnome

Re: RFR 8168840: InetAddress.getByName() throws java.net.UnknownHostException no such interface when used with virtual interfaces on Solaris

Re: RFR 8168840: InetAddress.getByName() throws java.net.UnknownHostException no such interface when used with virtual interfaces on Solaris

Re: RFR 8168840: InetAddress.getByName() throws java.net.UnknownHostException no such interface when used with virtual interfaces on Solaris

RFR(M): 8167420: Fixes for InetAddressImpl native coding on Linux/Unix platforms

RFR(M): 8167457: Fixes for InetAddressImpl native coding on Windows

Re: RFR:8170868: DefaultProxySelector should use system defaults on Windows, MacOS and Gnome

12 matches

Site Navigation

Mail list logo

Footer information