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