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

[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 incrementally with one additional commit since the last revision:

  unixdomainchannels: change to Net.c was missed after all

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

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

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

  Stats: 12 lines in 1 file changed: 0 ins; 10 del; 2 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