On Thu, 29 Jul 2021 08:12:23 GMT, Markus KARG <github.com+1701815+mk...@openjdk.org> wrote:
>> src/java.base/share/classes/sun/nio/ch/ChannelInputStream.java line 179: >> >>> 177: for (long n = srcSize - srcPos; bytesWritten < n;) >>> 178: bytesWritten += src.transferTo(srcPos + bytesWritten, >>> Long.MAX_VALUE, dest); >>> 179: return bytesWritten; >> >> If `src` is extended at an inconvenient point in time, for example between >> the return from a call to `src.transferTo()` that makes `bytesWritten < n` >> false and the evaluation of that terminating condition, then it appears that >> not all the content of `src` will be transferred to `dest`. I am not however >> sure that this violates any contract but it could be a behavioral change >> from the existing implementation. > > The JavaDocs in `InputStream::transferTo` *cleary* tell the caller that there > is **no** guarantee of *any* specific behavior in that particular case: >>The behavior for the case where the input and/or output stream is >>asynchronously closed, or the thread interrupted during the transfer, is >>highly input and output stream specific, and therefore not specified. Good point. >> src/java.base/share/classes/sun/nio/ch/ChannelOutputStream.java line 85: >> >>> 83: private byte[] bs; // Invoker's previous array >>> 84: private byte[] b1; >>> 85: >> >> It might be better to put the field declarations at the beginning of the >> class before the static methods. > > This is a question of style and personal likes. Code maintenance is much > easier if variables are defined *near* to its first use, not *far away* (as > in the Pascal or C language). If the reviewers want me to change it, I will > do it. It's actually a matter of convention but I think it can remain as it is. >> test/jdk/sun/nio/ch/ChannelInputStream/TransferTo.java line 67: >> >>> 65: test(readableByteChannelInput(), defaultOutput()); >>> 66: } >>> 67: >> >> This test looks like it's doing what it's supposed to do. I'm not asking to >> change it, but I think using TestNG might have given a simpler result with >> less work. For example, there could be one `DataProvider` supplying inputs >> which feed a `@Test` which expects an NPE, and another `DataProvider` >> supplying input-output pairs which feeds a `@Test` which validates the >> functionality. > > No doubt, using modern tools would have spared me work, and indeed I would > have chosen JUnit or TestNG if there would be a clear commitment to that tool > *upfront*. For now, I see now use in reworking the code afterwards. No need to rework it now. ------------- PR: https://git.openjdk.java.net/jdk/pull/4263
