fkjellberg commented on code in PR #418:
URL: https://github.com/apache/commons-io/pull/418#discussion_r1059464270
##########
src/main/java/org/apache/commons/io/HexDump.java:
##########
@@ -118,14 +148,54 @@ public static void dump(final byte[] data, final long
offset,
}
}
buffer.append(System.lineSeparator());
- // make explicit the dependency on the default encoding
- stream.write(buffer.toString().getBytes(Charset.defaultCharset()));
- stream.flush();
+ appendable.append(buffer);
buffer.setLength(0);
display_offset += chars_read;
}
}
+ /**
+ * Dumps an array of bytes to an OutputStream. The output is formatted
+ * for human inspection, with a hexadecimal offset followed by the
+ * hexadecimal values of the next 16 bytes of data and the printable ASCII
+ * characters (if any) that those bytes represent printed per each line
+ * of output.
+ * <p>
+ * The offset argument specifies the start offset of the data array
+ * within a larger entity like a file or an incoming stream. For example,
+ * if the data array contains the third kibibyte of a file, then the
+ * offset argument should be set to 2048. The offset value printed
+ * at the beginning of each line indicates where in that larger entity
+ * the first byte on that line is located.
+ * </p>
+ * <p>
+ * All bytes between the given index (inclusive) and the end of the
+ * data array are dumped.
+ * </p>
+ *
+ * @param data the byte array to be dumped
+ * @param offset offset of the byte array within a larger entity
+ * @param stream the OutputStream to which the data is to be
+ * written
+ * @param index initial index into the byte array
+ *
+ * @throws IOException is thrown if anything goes wrong writing
+ * the data to stream
+ * @throws ArrayIndexOutOfBoundsException if the index is
+ * outside the data array's bounds
+ * @throws NullPointerException if the output stream is null
+ */
+ public static void dump(final byte[] data, final long offset,
+ final OutputStream stream, final int index)
+ throws IOException, ArrayIndexOutOfBoundsException {
+ Objects.requireNonNull(stream, "stream");
+
+ try (OutputStreamWriter out = new
OutputStreamWriter(CloseShieldOutputStream.wrap(stream),
Charset.defaultCharset())) {
+ dump(data, offset, out, index, data.length - index);
+ out.flush();
Review Comment:
Commenting both on the `CloseShieldOutputStream` comment above and this
comment.
The original implementation flushed the underlying stream. It actually
flushed at every row written. Since the `OutputStreamWriter` will be
automatically closed within the try-with-resources block and looking at the
source code for `StreamEncoder` within the `OutputStreamWriter`, I now notice
that the `StreamEncoder` will flush before closing the stream. I will remove
the explicit flush in a follow up commit.
The original implementation left the `OutputStream` open after the call.
Since I wrap the `OutputStream` in an `OutputStreamWriter` that is closed, it
will propagate that close call to the underlying stream as well. I'm using the
`CloseShieldOutputStream` to protect it from being closed and preserve the same
behavior as before.
I think we should preserve the same behavior as the original code when it
comes to keeping the `OutputStream` open after the call.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
