RFR: JDK-8260966 (fs) Consolidate Linux and macOS implementations of UserDefinedFileAttributeView [v2]

[Alan Bateman]

On Wed, 17 Feb 2021 10:09:59 GMT, Sebastian Stenzel <github.com+1204330+overheadhunter at openjdk.org> wrote:

>> Deduplicated code in these classes:
>> - `LinuxUserDefinedAttributeView` + `BsdUserDefinedAttributeView` → `UnixUserDefinedAttributeView`. Due to different supported length of attribute names, I added an abstract method `UnixUserDefinedAttributeView.maxNameLength()`.
>> - `LinuxNativeDispatcher` + `BsdNativeDispatcher` → `UnixNativeDispatcher`. I basically just moved the Linux implementation up to Unix and added preprocessor directives to distinguish Linux/BSD/Other. Others will throw ENOTSUP UnixExceptions.
>> - `LinuxFileStore.isExtendedAttributesEnabled()` + `BsdFileStore.isExtendedAttributesEnabled()` → `UnixFileStore.isExtendedAttributesEnabled()`
>> 
>> For the latter I introduced `UnixConstants.XATTR_NOT_FOUND`, which is `ENODATA` on Linux and `ENOATTR` on BSD. Note that `UnixConstants.ENODATA` is still present, as @AlanBateman "would prefer if we left ENODATA so that it can be used in Linux specific code" (Quote from https://github.com/openjdk/jdk/pull/2363#issuecomment-772534587)
>> 
>> This also fixes `Files.copy(src, dst, StandardCopyOption.COPY_ATTRIBUTES)` on macOS/BSD. Previosly an [invocation was missing](https://github.com/openjdk/jdk/blob/jdk-17%2B7/src/java.base/macosx/classes/sun/nio/fs/BsdFileSystem.java#L70-L72).
>> 
>> I plan to add further commits to clean up this code, once the deduplication is reviewed.
>
> Sebastian Stenzel has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Restored indentation from former LinuxUserDefinedFileAttributeView and LinuxNativeDispatcher

Overall I think this looks good, just a few minor nits with the method descriptions on protected methods (they don't actually need to be protected of course).

src/java.base/unix/classes/sun/nio/fs/UnixUserDefinedFileAttributeView.java line 49:

> 47: 
> 48:     // returns the maximum supported length of xattr names (in bytes, including namespace)
> 49:     protected abstract int maxNameLength();

I think we should move this declaration to after the constructor to avoid having an abstract method declared in the middle of private API elements. I'd probably use /** .. */ style for the method description as it's not private.

src/java.base/unix/classes/sun/nio/fs/UnixFileStore.java line 177:

> 175:     // returns true if extended attributes enabled on file system where given
> 176:     // file resides, returns false if disabled or unable to determine.
> 177:     protected boolean isExtendedAttributesEnabled(UnixPath path) {

This used to be private method in LinuxFileStore, now it's a protected method so might be cleaner to change the method description to use /** .. */ comment as it's used from sub-classes.

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

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