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