RFR: 6980847: (fs) Files.copy needs to be tuned [v7]
Brian Burkhalter
bpb at openjdk.org
Wed Jun 22 15:39:03 UTC 2022
On Wed, 22 Jun 2022 14:34:40 GMT, Alan Bateman <alanb at openjdk.org> wrote:
>> Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
>>
>> 6980847: Cleanup
>
> src/java.base/unix/classes/sun/nio/fs/UnixCopyFile.java line 233:
>
>> 231: private static long lcm(long x, long y) {
>> 232: if (x <= 0 || y <= 0)
>> 233: throw new IllegalArgumentException("Non-positive parameter");
>
> If lcm were to be used more widely then throwing IAE would make sense. However, the usage here is private to the copy method and it only calls lcm when x and y are positive. It also catches IAE. It might be simpler to just put an assert x > 0 && y > 0 and remove the catch of IAE. That would catch any bugs in this code.
So changed in 6c7ac10c4a98e0d11cb487dfe816f460f7381002.
> src/java.base/unix/classes/sun/nio/fs/UnixCopyFile.java line 330:
>
>> 328: } finally {
>> 329: Util.releaseTemporaryDirectBuffer(buf);
>> 330: Blocker.end(comp);
>
> It might be better to use a nested try-finally to ensure that the buffer get/release and blocker begin/end are symmetric.
So changed in 6c7ac10c4a98e0d11cb487dfe816f460f7381002.
> src/java.base/unix/classes/sun/nio/fs/UnixCopyFile.java line 733:
>
>> 731: * (a non-zero value written to this address indicates cancel)
>> 732: */
>> 733: private static native void bufferCopy0(int dst, int src, long address,
>
> It might be clearer if the name were bufferedCopy rather than bufferCopy.
So changed in 6c7ac10c4a98e0d11cb487dfe816f460f7381002.
-------------
PR: https://git.openjdk.org/jdk/pull/9161
More information about the nio-dev
mailing list