RFR: JDK-8264110 (fs) possible UnsupportedOperationException in UnixUserDefinedFileAttributeView.read(...)

[Sebastian Stenzel]

On Fri, 26 Mar 2021 16:56:15 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> During `write()`, we check whether the `src` buffer exposes a raw byte array:
>> 
>> https://github.com/openjdk/jdk/blob/4e74de4b2eec611b49ee8defae1ab06351280008/src/java.base/unix/classes/sun/nio/fs/UnixUserDefinedFileAttributeView.java#L247-L258
>> 
>> The same is now also done during `read()`. This should allow using arbitrary ByteBuffer implementations as `dst` and restores a certain symmetry between read and write.
>
> src/java.base/unix/classes/sun/nio/fs/UnixUserDefinedFileAttributeView.java line 197:
> 
>> 195:                     unsafe.copyMemory(null, address, tmp, 0, n);
>> 196:                     dst.put(tmp);
>> 197:                 }
> 
> The changes look okay but I'm wondering what cases you have in mind so that we can add tests. We lack tests that use of these APIs with buffers that are created with JNI NewDirectByteBuffer.

Tbh, I don't know any specific ByteBuffer implementation (other than DirectByteBuffer) that doesn't have an array, but in theory it is explicitly allowed. And since this `read(String name, ByteBuffer dst)` is public API, it should be prepared to deal with any allowed params.

But after all I didn't encounter any exception, I just copied the implementation from `write()` as I deemed it reasonable.

Alternatively, if ByteBuffers were sealed classes and only allowed HeapByteBuffers (known to have an array) or DirectByteBuffers (allowing direct access to the address, thus irrelevant in this part of the code) we could skip the distinction entirely. But I guess that isn't something we can decide in the scope of this PR.

Regarding test cases: Other than creating a ByteBuffer mock that returns false upon `hasArray()`, I don't know how to verify this branch. That said, so far I failed to understand what framework or pattern to use to write JDK tests. The UserDefinedAttributeView test seems to be a mere `main(String[] args)` class that must not complete exceptionally.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3217