Updating existing JDK code to use InputStream.transferTo() (original) (raw)

Pavel Rappo pavel.rappo at oracle.com
Wed May 13 15:29:04 UTC 2015

It now can be reviewed as usual at:

http://cr.openjdk.java.net/~prappo/8080272/webrev.00/ Feel free to review. Thanks.

Let me start then.

  1. I've seen several cases where current behaviour

    while ((n = in.read(buffer)) > 0) ~~~

has been changed to

while ((read = this.read(buffer, 0, TRANSFER_BUFFER_SIZE)) >= 0)
                                                           ~~~

Those things jump out at you, but don't seem to be harmful, since the only case where:

java.io.InputStream.read(byte[], int off, int len)
java.io.InputStream#read(byte[])

may return 0, is when len == 0. And it's never the case here. Unless, of course, some misbehaving implementation of InputStream is used.

  1. jdk/src/java.xml.crypto/share/classes/com/sun/org/apache/xml/internal/security/utils/JavaUtils.java

Some of the refactored methods there seem to be unused:

getBytesFromFile(String)
writeBytesToFilename(String, byte[])

Should we remove them instead?

  1. Thanks to Patrick I've learned

    AutoCloseable s = ... try (s) { ~~~ }

is not a bug, but new javac 9 feature [1]. Learned it the hard way.

  1. I wonder if javax.management.loading.MLet.loadLibraryAsResource and sun.net.www.MimeLauncher.run could be refactored with more appropriate

    Files.copy(is, file.toPath())

as it seems to fit there perfectly.

  1. Now we don't need the com.sun.media.sound.StandardMidiFileWriter.bufferSize after we removed its sole client.

  2. I've run into several cases where code use explicit flush() for ByteArrayOutputStream, BufferedOutputStream. For the former one it's never needed. The latter one does it automatically on close(). Should we still call them?

  3. org.jcp.xml.dsig.internal.dom.Utils.readBytesFromStream is a little bit awkward since it used 1024 as some sort of a cut-off. It might've had something to do with EOF detection, but I'm not sure.

  4. jdk/src/java.desktop/windows/classes/sun/print/Win32PrintJob.java

    } catch (FileNotFoundException fnfe) { notifyEvent(PrintJobEvent.JOB_FAILED); throw new PrintException(fnfe.toString()); } catch (IOException ioe) { notifyEvent(PrintJobEvent.JOB_FAILED); throw new PrintException(ioe.toString()); }

It seems ok to remove the FileNotFoundException branch. Or I'm missing something.

  1. I don't think I would agree with some changes in sun.net.www.MimeLauncher.run as they obviously make the new version not equivalent to the old one. Non swallowed IOException, additional null check. Way too much for the cleanup change.

Summary

The change seems ok to me, given that (7, 9) are justified or fixed. Patrick needs a _R_eviewer for his changes to be accepted. Please help him with this.

[1] https://blogs.oracle.com/darcy/entry/concise_twr_jdk9

More information about the core-libs-dev mailing list