Re: RFR: 8286781: Replace the deprecated/obsolete gethostbyname and inet_addr calls [v2]

2023-03-03 Thread David Holmes
On Fri, 3 Mar 2023 07:30:39 GMT, Kim Barrett  wrote:

>> David Holmes has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Restrict getaddrinfo to IPv4 only as per the rest of the code
>
> Looks good.

Thanks for the reviews @kimbarrett  and @djelinski ! I'll leave this till early 
next week to integrate.

-

PR: https://git.openjdk.org/jdk/pull/12842


Re: RFR: 8286781: Replace the deprecated/obsolete gethostbyname and inet_addr calls [v2]

2023-03-03 Thread Daniel JeliƄski
On Fri, 3 Mar 2023 09:56:35 GMT, David Holmes  wrote:

>> We can replace `gethostbyname`, which is deprecated on Windows and Linux, 
>> with `getaddrinfo`. This API is available on all supported platforms and so 
>> can be placed in shared code. @djelinski pointed out that `getaddrinfo` can 
>> resolve both IP addresses and host names so the two step approach used in 
>> `networkStream::connect` is not necessary and we can do away with 
>> `os::get_host_by_name()` completely.
>> 
>> The build is updated to enable winsock deprecation warnings, and now we need 
>> to use `ws2_32.lib` we can drop `wsock32.lib` (as it is basically a subset - 
>> again thanks @djelinski ).
>> 
>> Testing
>> - all Oracle builds in tiers 1-5
>> - All GHA builds
>> 
>> The actual code change has to be manually tested because the code is only 
>> used by Ideal Graph Printing to connect to the Ideal Graph Visualizer. I've 
>> manually tested on Windows and Linux and @tobiasholenstein tested macOS.
>> 
>> Thanks.
>
> David Holmes has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Restrict getaddrinfo to IPv4 only as per the rest of the code

Marked as reviewed by djelinski (Committer).

-

PR: https://git.openjdk.org/jdk/pull/12842


Re: RFR: 8286781: Replace the deprecated/obsolete gethostbyname and inet_addr calls [v2]

2023-03-03 Thread David Holmes
> We can replace `gethostbyname`, which is deprecated on Windows and Linux, 
> with `getaddrinfo`. This API is available on all supported platforms and so 
> can be placed in shared code. @djelinski pointed out that `getaddrinfo` can 
> resolve both IP addresses and host names so the two step approach used in 
> `networkStream::connect` is not necessary and we can do away with 
> `os::get_host_by_name()` completely.
> 
> The build is updated to enable winsock deprecation warnings, and now we need 
> to use `ws2_32.lib` we can drop `wsock32.lib` (as it is basically a subset - 
> again thanks @djelinski ).
> 
> Testing
> - all Oracle builds in tiers 1-5
> - All GHA builds
> 
> The actual code change has to be manually tested because the code is only 
> used by Ideal Graph Printing to connect to the Ideal Graph Visualizer. I've 
> manually tested on Windows and Linux and @tobiasholenstein tested macOS.
> 
> Thanks.

David Holmes has updated the pull request incrementally with one additional 
commit since the last revision:

  Restrict getaddrinfo to IPv4 only as per the rest of the code

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/12842/files
  - new: https://git.openjdk.org/jdk/pull/12842/files/81f15e05..8b6f6317

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=12842=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=12842=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/12842.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/12842/head:pull/12842

PR: https://git.openjdk.org/jdk/pull/12842