RFR: JDK-8260966 (fs) Consolidate Linux and macOS implementations of UserDefinedFileAttributeView
Sebastian Stenzel
github.com+1204330+overheadhunter at openjdk.java.net
Tue Feb 2 21:27:59 UTC 2021
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
-------------
Commit messages:
- Deduplicated code in LinuxFileStore and BsdFileStore
- further deduplication and complexity reduction
- deduplicate LinuxUserDefinedAttributeView and BsdUserDefinedAttributeView
Changes: https://git.openjdk.java.net/jdk/pull/2363/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2363&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8260966
Stats: 1510 lines in 17 files changed: 520 ins; 971 del; 19 mod
Patch: https://git.openjdk.java.net/jdk/pull/2363.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/2363/head:pull/2363
PR: https://git.openjdk.java.net/jdk/pull/2363
More information about the nio-dev
mailing list