RFR: 8170769 Provide a simple hexdump facility for binary data

Alan Bateman Alan.Bateman at oracle.com
Tue Dec 11 13:04:24 UTC 2018


On 08/12/2018 01:18, Vincent Ryan wrote:
> Here’s the latest version that addresses all the current open issues:
>
> webrev: http://cr.openjdk.java.net/~vinnie/8170769/webrev.07/ <http://cr.openjdk.java.net/~vinnie/8170769/webrev.07/>
> javadoc: http://cr.openjdk.java.net/~vinnie/8170769/javadoc.07/api/java.base/java/util/HexFormat.html <http://cr.openjdk.java.net/~vinnie/8170769/javadoc.07/api/java.base/java/util/HexFormat.html>
>
> CSR: https://bugs.openjdk.java.net/browse/CCC-8170769 <https://bugs.openjdk.java.net/browse/CCC-8170769>
>
I read through the latest javadoc corresponding to webrev.07.

Overall I think it looks very good except for dumpAsStream(ByteBuffer, 
fromIndex, toIndex, chunkSize, Formatter) as it's not clear if fromIndex 
is from the buffer position or an absolute index. If the former then 
there are a couple of other issues. I see Roger has touched on the 
advancement of the buffer position to its limit which isn't right unless 
all remaining bytes in the  buffer are consumer. There is also an issue 
with the exception as attempting to consume beyond the limit is a 
BufferUnderflowException. It might be simpler to replace this method 
with a dumpAsStream(ByteBuffer, chunkSize, Formatter) that can lazily 
(rather than eagerly) consume the remaining bytes. Additional overloads 
could be added in the future if needed.

dumpAsStream(InputStream) specifies "On return, the input stream will be 
at end-of-stream" but I assume this isn't right as the method is lazy.

The 3-arg dumpAsStream doesn't specify the exception/behavior for when 
chunkSize is <= 0.

fromString(CharSequence) may need clarification on how it behaves when 
the CS is a CharBuffer. Does it consume all the remaining chars in the 
buffer so its position be advanced to the limit? The 3-arg version is 
also not clear on this point.

In Formatter interface it may be useful to expand the description of the 
"offset" parameter as readers may not immediately understand that it's 
just an input for the formatted string rather than a real offset, an 
example might help. It also doesn't say if the offset can be specified 
as <= 0L or not.

A minor comment is that several places refer to a byte array as a "byte 
buffers. Is this deliberate or can these be replaced with "byte array" 
to avoid confusion. Another minor comment is that one of the 
dumpAsStream methods is missing @throws NPE - maybe they should all be 
removed and replaced with a statement in the class description.

-Alan


More information about the core-libs-dev mailing list