RFR: 8293502: (fc) FileChannel::transfer methods fail to copy /proc files on Linux [v2]

Brian Burkhalter bpb at openjdk.org
Thu Sep 15 23:07:50 UTC 2022


On Thu, 15 Sep 2022 10:59:20 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8293502: Use /proc/cpuinfo instead of /proc/mounts and other cleanup
>
> src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java line 774:
> 
>> 772:         // Adjust count only if remaining > 0, i.e.,
>> 773:         // sz > position which means sz > 0
>> 774:         if (remaining < count && remaining > 0)
> 
> It might be a bit more readable to swap this to test `remaining > 0 && remaining < count`.

Fixed in 959011949525bde73f36a702b59d1e27bec569c6.

> test/jdk/java/nio/file/Files/CopyProcFile.java line 80:
> 
>> 78:     static long copy(String src, String dst) {
>> 79:         try {
>> 80:             return Files.size(Files.copy(Path.of(src), Path.of(dst)));
> 
> I think it might be clearer if you split this into
> 
> Path target = Files.copy(Path.of(src), Path.of(dst));
> return Files.size(target);
> 
> only because it might not be obvious that it's the target Path that is returned.

Fixed in 959011949525bde73f36a702b59d1e27bec569c6.

> test/jdk/java/nio/file/Files/CopyProcFile.java line 100:
> 
>> 98:         try (FileChannel fci = FileChannel.open(Path.of(src), READ);
>> 99:              FileChannel fco = FileChannel.open(Path.of(dst), CREATE_NEW,
>> 100:                                                 WRITE);) {
> 
> I don't know if this line break is intentional or not but it would be more readable if WRITE were moved to the previous line.
> 
> Update: Same thing in transferFrom.

Fixed in 959011949525bde73f36a702b59d1e27bec569c6.

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

PR: https://git.openjdk.org/jdk/pull/10267


More information about the nio-dev mailing list