On Wed, 20 Apr 2022 16:52:31 GMT, XenoAmess <d...@openjdk.java.net> wrote:
>> @jmehrens what about this then?
>> I think it safe now(actually this mechanism is learned from Reader)
>
> XenoAmess has updated the pull request incrementally with one additional
> commit since the last revision:
>
> add documents
src/java.base/share/classes/java/io/InputStream.java line 72:
> 70: *
> 71: * @param size minimum length that the skip byte array must have.
> 72: * notice that this param input MUST be equal or less
> than {@link #MAX_SKIP_BUFFER_SIZE MAX_SKIP_BUFFER_SIZE}.
The "MUST be equal" reads like something will go wrong if that condition isn't
satisfied.
This method does not care about the size, except in potentially resizing if the
requested size is larger.
The "notice..." statement is making a statement about code in the caller, and
so doesn't belong here.
src/java.base/share/classes/java/io/InputStream.java line 75:
> 73: * @return the byte array.
> 74: */
> 75: private byte[] skipBufferReference(int size) {
The method name would match the return value better if named `skipBuffer(size)`.
src/java.base/share/classes/java/io/InputStream.java line 78:
> 76: SoftReference<byte[]> ref = this.skipBufferReference;
> 77: byte[] buffer;
> 78: if (ref == null || (buffer = ref.get()) == null || buffer.length
> < size) {
A comment here or in the method javadoc to the effect that a new buffer is
allocated unless the existing buffer is large enough might head off doubt that
buffer is always non-null at the return. As would inverting the if:
if (ref != null && (buffer = ref.get()) != null && buffer.length >= size) {
return buffer;
}
// allocate new or larger buffer
buffer = new byte[size];
this.skipBufferReference = new SoftReference<>(buffer);
return buffer;
-------------
PR: https://git.openjdk.java.net/jdk/pull/5872
