RFR: JDK-8262957 (fs) Fail fast in UnixFileStore.isExtendedAttributesEnabled
Alan Bateman
alanb at openjdk.java.net
Thu Mar 4 20:28:40 UTC 2021
On Thu, 4 Mar 2021 15:27:56 GMT, Sebastian Stenzel <github.com+1204330+overheadhunter at openjdk.org> wrote:
>> src/java.base/unix/classes/sun/nio/fs/UnixFileStore.java line 186:
>>
>>> 184: return false;
>>> 185: }
>>> 186:
>>
>> Add SUPPORTS_XATTR is a good idea.
>> Style-wise, it might be a bit neater to use if (UnixNativeDispatcher.xattrSupported()) { ...} so there will only one "return false" code path.
>> Looking at this code now make me wonder about the close(fd) for the case that the open fails. It should be checking if fd or there should be a nested try-finally so that close is only invoked when the open succeeds.
>
>> Style-wise, it might be a bit neater to use if (UnixNativeDispatcher.xattrSupported()) { ...} so there will only one "return false" code path.
>
> I prefer to avoid nested blocks for improved readability, but thats mostly a matter of taste. If you like, I can reverse the "if".
>
>> Looking at this code now make me wonder about the close(fd) for the case that the open fails. It should be checking if fd or there should be a nested try-finally so that close is only invoked when the open succeeds.
>
> True. But I guess that would be off-topic in this PR?
It's somewhat subjective. What you have is fine, I just would have done is differently to avoid too many return paths.
I think we should clean up the close(-1) case while we are in the area, separate PR of course.
-------------
PR: https://git.openjdk.java.net/jdk/pull/2816
More information about the nio-dev
mailing list