RFR: 8245194: Unix domain socket channel implementation [v11]

[Michael McMahon]

> Continuing this review as a PR on github with the comments below incorporated. I expect there will be a few more
> iterations before integrating.
> On 06/09/2020 19:47, Alan Bateman wrote:
>> On 26/08/2020 15:24, Michael McMahon wrote:
>>>
>>> As I mentioned the other day, I wasn't able to use the suggested method on Windows which returns an absolute path. So,
>>> I added a method to WindowsPath which returns the path in the expected UTF-8 encoding as a byte[]. Let me know what you
>>> think of that.
>>>
>>> There is a fair bit of other refactoring and simplification done also. Next version is at:
>>>
>>> http://cr.openjdk.java.net/~michaelm/8245194/impl.webrev/webrev.9/
>>>
>> Adding a method to WindowsPath to encode the path using UTF-8 is okay but I don't think we should be caching it as the
>> encoding for sun_path is an outlier on Windows. I'm also a bit dubious about encoding a relative path when the resolved
>> path (before encoding) is > 247 chars. The documentation on the MS site isn't very completely and I think there are a
>> number points that require clarification to fully understand how this will work with relative, directly relative and
>> drive relative paths.
>>
> 
> Maybe it would be better to just use the path returned from toString() and do the conversion to UTF-8 externally. That
> would leave WindowsPath unchanged.
>> In the same area, the new PathUtil is a bit inconsistent with the existing provider code. One suggestion is to add a
>> method to AbstractFileSystemProvider instead. That is the base class the platform providers and would be a better place
>> to get the file path in bytes.
>>
> 
> Okay, I gave the method a name that is specific to Unix domain sockets because this UTF-8 strangeness is not likely to
> be useful by other components.
>> One other comment on the changes to the file system provider it should be okay to change UnixUserPrinipals ad its
>> fromUid and fromGid method to be public. This would mean that the patch shouldn't need to add UnixUserGroup (the main
>> issue is that class is that it means we end up with two classes with static factory methods doing the same thing).
> 
> Okay, that does simplify it a bit.
> 
> Thanks,
> Michael.
> 
>> -Alan.
>>
>>
>>
>>
>>
>>

Michael McMahon has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev
excludes the unrelated changes brought in by the merge/rebase. The pull request contains 13 additional commits since
the last revision:

 - Merge branch 'master' into unixdomainchannels
 - unixdomainchannels: remove more cruft from Windows Net.c
 - unixdomainchannels: change to Net.c was missed after all
 - unixdomainchannels: typo in WindowsFileSystemProvider.java
 - unixdomainchannels: Update after Alan's review of Sept 29
 - Merge branch 'master' into unixdomainchannels
 - unixdomainchannels: some tests were failing on Windows version prior to 10/2019
 - unixdomainchannels: updates after comments sent by Alan 14 Sept
 - unixdomainchannels: updates from review of Sept 11 2020
 - Merge branch 'master' into unixdomainchannels
 - ... and 3 more: https://git.openjdk.java.net/jdk/compare/39516fa7...c40d2059

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/52/files
  - new: https://git.openjdk.java.net/jdk/pull/52/files/3fe6baa4..c40d2059

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=52&range=10
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=52&range=09-10

  Stats: 30349 lines in 734 files changed: 5895 ins; 22231 del; 2223 mod
  Patch: https://git.openjdk.java.net/jdk/pull/52.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/52/head:pull/52

PR: https://git.openjdk.java.net/jdk/pull/52