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