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
- Previous message: Updating existing JDK code to use InputStream.transferTo()
- Next message: Updating existing JDK code to use InputStream.transferTo()
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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.
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.
- 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?
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.
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.
Now we don't need the com.sun.media.sound.StandardMidiFileWriter.bufferSize after we removed its sole client.
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 onclose()
. Should we still call them?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.
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.
- 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
- Previous message: Updating existing JDK code to use InputStream.transferTo()
- Next message: Updating existing JDK code to use InputStream.transferTo()
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]