RFR: JDK-8260966 (fs) Consolidate Linux and macOS implementations of UserDefinedFileAttributeView
Sebastian Stenzel
github.com+1204330+overheadhunter at openjdk.java.net
Wed Feb 3 12:08:42 UTC 2021
On Tue, 2 Feb 2021 21:23:04 GMT, Sebastian Stenzel <github.com+1204330+overheadhunter at openjdk.org> wrote:
> I want to discuss the changes of this PR in three sections:
>
> # Deduplications
> The main focus was to deduplicate code:
> - `LinuxUserDefinedAttributeView` + `BsdUserDefinedAttributeView` → `UnixUserDefinedAttributeView`. Due to different supported length of attribute names, I made added an abstract method `UnixUserDefinedAttributeView. maxNameLength()` and kept the other two.
> - `LinuxNativeDispatcher` + `BsdNativeDispatcher` → `UnixNativeDispatcher`. I basically just moved the Linux implementation up to Unix. BSD-specific adjustments are now done in C.
> - `LinuxFileStore.isExtendedAttributesEnabled()` + `BsdFileStore.isExtendedAttributesEnabled()` → `UnixFileStore.isExtendedAttributesEnabled()`
> - For the latter I hade to combine `UnixConstants.ENODATA` and `UnixConstants.ENOATTR` to a platform-agnostic `UnixConstants.XATTR_NOT_FOUND` ⚠️ Warning: While `ENODATA` is no longer used JDK-internally, it might have been used via reflection in downstream projects, thus causing regressions. Maintainers should decide, if we might want to keep `ENODATA` additionally to the new `XATTR_NOT_FOUND`.
> - Deduplicated code inside of *NativeDispatcher's methods `copyExtendedAttributes(int ofd, int nfd)` and `list()`
>
> # Cleanup / Improvements
> - On AIX (or other Unix Platforms not explicitly supporting xattr), calling related native methods will result in `ENOTSUP`
> - `NativeBuffer implements AutoCloseable` and used try-with-resource statements where applicable
> - changed loop to recursive function for creation of larger buffer, if `flistxattr` fails due to `ERANGE` error (max recursion depth is 5, I don't expect performance benefits from loop unrolling, therefore chose reduced cyclomatic complexity over performance)
> - Added `UnixNativeDispatcher.SUPPORTS_XATTR` capability and added a shortcut to `UnixFileStore. isExtendedAttributesEnabled(...)` to skip I/O that is known to fail
> - Removed legacy `dlsym` for xattr-related Linux system calls (fixes JDK-8260691)
>
> # Fixes
> - `Files.copy(src, dst, StandardCopyOption.COPY_ATTRIBUTES)` now correctly copies xattr on macOS/BSD ([invocation was missing before](https://github.com/openjdk/jdk/blob/jdk-17%2B7/src/java.base/macosx/classes/sun/nio/fs/BsdFileSystem.java#L70-L72))
> - no longer [access `dst.arrayOffset()` without checking](https://github.com/openjdk/jdk/blob/jdk-17+7/src/java.base/linux/classes/sun/nio/fs/LinuxUserDefinedFileAttributeView.java#L201) if `dst.hasArray()` during `read()`. Code is now symmetric with `write()`
> - fixed [potential resource leak of `nb` during `write()`](https://github.com/openjdk/jdk/blob/jdk-17+7/src/java.base/linux/classes/sun/nio/fs/LinuxUserDefinedFileAttributeView.java#L258), if `file.openForAttributeAccess(...)` fails
>
>
> Please note, that while the tests succeed on macOS, so far I didn't manage to run `jtreg:test/jdk/java/nio/file/attribute/` on a Linux CI system due to lack of knowledge or will power
Ok I managed to get jtreg running in a Linux container now. Looks good:
==============================
Test summary
==============================
TEST TOTAL PASS FAIL ERROR
jtreg:test/jdk/java/nio/file/attribute 10 10 0 0
==============================
TEST SUCCESS
Finished building target 'run-test' in configuration 'linux-x86_64-server-release'
-------------
PR: https://git.openjdk.java.net/jdk/pull/2363
More information about the nio-dev
mailing list