You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2022/12/29 14:46:37 UTC

[GitHub] [commons-io] fkjellberg opened a new pull request, #418: Add support for Appendable to HexDump util

fkjellberg opened a new pull request, #418:
URL: https://github.com/apache/commons-io/pull/418

   This PR adds support to write the output from HexDump utility to an `Appendable` making it possible to use e.g. `System.out` or `StringBuilder` as output targets.


-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-io] fkjellberg commented on a diff in pull request #418: [IO-784] Add support for Appendable to HexDump util

Posted by GitBox <gi...@apache.org>.
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-io] garydgregory commented on a diff in pull request #418: [IO-784] Add support for Appendable to HexDump util

Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #418:
URL: https://github.com/apache/commons-io/pull/418#discussion_r1059365488


##########
src/main/java/org/apache/commons/io/HexDump.java:
##########
@@ -53,7 +56,26 @@ public class HexDump {
             };
 
     /**
-     * Dumps an array of bytes to an OutputStream. The output is formatted
+     * Dumps an array of bytes to an Appendable. 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.
+     *
+     * @param data  the byte array to be dumped
+     * @param out  the Appendable to which the data is to be written
+     *
+     * @throws IOException is thrown if anything goes wrong writing
+     *         the data to appendable
+     */
+

Review Comment:
   Remove whitespace between Javadoc and code.



##########
src/main/java/org/apache/commons/io/HexDump.java:
##########
@@ -53,7 +56,26 @@ public class HexDump {
             };
 
     /**
-     * Dumps an array of bytes to an OutputStream. The output is formatted
+     * Dumps an array of bytes to an Appendable. 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.
+     *
+     * @param data  the byte array to be dumped
+     * @param out  the Appendable to which the data is to be written
+     *
+     * @throws IOException is thrown if anything goes wrong writing
+     *         the data to appendable
+     */
+

Review Comment:
   Add Javadoc since tag to new public and protected elements. 



##########
src/main/java/org/apache/commons/io/HexDump.java:
##########
@@ -53,7 +56,26 @@ public class HexDump {
             };
 
     /**
-     * Dumps an array of bytes to an OutputStream. The output is formatted
+     * Dumps an array of bytes to an Appendable. 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.
+     *
+     * @param data  the byte array to be dumped
+     * @param out  the Appendable to which the data is to be written
+     *
+     * @throws IOException is thrown if anything goes wrong writing
+     *         the data to appendable
+     */
+
+    public static void dump(final byte[] data, final Appendable out)

Review Comment:
   Rename `out` to `appendable` so it has no chance of being confused with an Output* class.



##########
src/main/java/org/apache/commons/io/HexDump.java:
##########
@@ -118,14 +144,55 @@ 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();
+            out.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");
+
+        // make explicit the dependency on the default encoding

Review Comment:
   This comment is not needed: "make explicit the dependency on the default encoding"



-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-io] fkjellberg commented on a diff in pull request #418: [IO-784] Add support for Appendable to HexDump util

Posted by GitBox <gi...@apache.org>.
fkjellberg commented on code in PR #418:
URL: https://github.com/apache/commons-io/pull/418#discussion_r1059394620


##########
src/main/java/org/apache/commons/io/HexDump.java:
##########
@@ -118,14 +144,55 @@ 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();
+            out.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");
+
+        // make explicit the dependency on the default encoding

Review Comment:
   This comment came from the original code. I've removed it now.



##########
src/main/java/org/apache/commons/io/HexDump.java:
##########
@@ -53,7 +56,26 @@ public class HexDump {
             };
 
     /**
-     * Dumps an array of bytes to an OutputStream. The output is formatted
+     * Dumps an array of bytes to an Appendable. 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.
+     *
+     * @param data  the byte array to be dumped
+     * @param out  the Appendable to which the data is to be written
+     *
+     * @throws IOException is thrown if anything goes wrong writing
+     *         the data to appendable
+     */
+
+    public static void dump(final byte[] data, final Appendable out)

Review Comment:
   Fixed



-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-io] garydgregory commented on pull request #418: [IO-784] Add support for Appendable to HexDump util

Posted by GitBox <gi...@apache.org>.
garydgregory commented on PR #418:
URL: https://github.com/apache/commons-io/pull/418#issuecomment-1367573842

    Code coverage is negative with this PR, so you are likely missing some code paths in the new code.


-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-io] codecov-commenter commented on pull request #418: [IO-784] Add support for Appendable to HexDump util

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #418:
URL: https://github.com/apache/commons-io/pull/418#issuecomment-1367441946

   # [Codecov](https://codecov.io/gh/apache/commons-io/pull/418?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#418](https://codecov.io/gh/apache/commons-io/pull/418?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2b60f3a) into [master](https://codecov.io/gh/apache/commons-io/commit/3bd9659002d88405a60e21231cca78c3da7a7ecc?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3bd9659) will **decrease** coverage by `0.04%`.
   > The diff coverage is `92.85%`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #418      +/-   ##
   ============================================
   - Coverage     86.11%   86.07%   -0.05%     
   - Complexity     3212     3214       +2     
   ============================================
     Files           215      215              
     Lines          7496     7503       +7     
     Branches        906      907       +1     
   ============================================
   + Hits           6455     6458       +3     
   - Misses          794      796       +2     
   - Partials        247      249       +2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/commons-io/pull/418?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [src/main/java/org/apache/commons/io/HexDump.java](https://codecov.io/gh/apache/commons-io/pull/418/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvaW8vSGV4RHVtcC5qYXZh) | `93.47% <92.85%> (-1.12%)` | :arrow_down: |
   | [.../main/java/org/apache/commons/io/input/Tailer.java](https://codecov.io/gh/apache/commons-io/pull/418/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvaW8vaW5wdXQvVGFpbGVyLmphdmE=) | `85.57% <0.00%> (-1.50%)` | :arrow_down: |
   | [...ache/commons/io/input/ReversedLinesFileReader.java](https://codecov.io/gh/apache/commons-io/pull/418/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvaW8vaW5wdXQvUmV2ZXJzZWRMaW5lc0ZpbGVSZWFkZXIuamF2YQ==) | `87.12% <0.00%> (-0.20%)` | :arrow_down: |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-io] fkjellberg commented on pull request #418: [IO-784] Add support for Appendable to HexDump util

Posted by GitBox <gi...@apache.org>.
fkjellberg commented on PR #418:
URL: https://github.com/apache/commons-io/pull/418#issuecomment-1367794115

   @garydgregory I added one more test and rebased the branch


-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-io] fkjellberg commented on a diff in pull request #418: [IO-784] Add support for Appendable to HexDump util

Posted by GitBox <gi...@apache.org>.
fkjellberg commented on code in PR #418:
URL: https://github.com/apache/commons-io/pull/418#discussion_r1064184381


##########
src/test/java/org/apache/commons/io/HexDumpTest.java:
##########
@@ -253,6 +253,16 @@ public void testDumpOutputStream() throws IOException {
 
         // verify proper behavior with null stream
         assertThrows(NullPointerException.class, () -> HexDump.dump(testArray, 0x10000000, null, 0));
+

Review Comment:
   @garydgregory Thanks for the pointer. I was not aware of this test utility class. Code updated.



-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-io] garydgregory commented on a diff in pull request #418: [IO-784] Add support for Appendable to HexDump util

Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #418:
URL: https://github.com/apache/commons-io/pull/418#discussion_r1064170937


##########
src/test/java/org/apache/commons/io/HexDumpTest.java:
##########
@@ -253,6 +253,16 @@ public void testDumpOutputStream() throws IOException {
 
         // verify proper behavior with null stream
         assertThrows(NullPointerException.class, () -> HexDump.dump(testArray, 0x10000000, null, 0));
+

Review Comment:
   How about reusing `ThrowOnCloseInputStream`?



-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-io] fkjellberg commented on a diff in pull request #418: [IO-784] Add support for Appendable to HexDump util

Posted by GitBox <gi...@apache.org>.
fkjellberg commented on code in PR #418:
URL: https://github.com/apache/commons-io/pull/418#discussion_r1059394498


##########
src/main/java/org/apache/commons/io/HexDump.java:
##########
@@ -53,7 +56,26 @@ public class HexDump {
             };
 
     /**
-     * Dumps an array of bytes to an OutputStream. The output is formatted
+     * Dumps an array of bytes to an Appendable. 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.
+     *
+     * @param data  the byte array to be dumped
+     * @param out  the Appendable to which the data is to be written
+     *
+     * @throws IOException is thrown if anything goes wrong writing
+     *         the data to appendable
+     */
+

Review Comment:
   @garydgregory Thanks for reviewing. I've fixed both.



-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-io] garydgregory merged pull request #418: [IO-784] Add support for Appendable to HexDump util

Posted by GitBox <gi...@apache.org>.
garydgregory merged PR #418:
URL: https://github.com/apache/commons-io/pull/418


-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-io] garydgregory commented on pull request #418: [IO-784] Add support for Appendable to HexDump

Posted by GitBox <gi...@apache.org>.
garydgregory commented on PR #418:
URL: https://github.com/apache/commons-io/pull/418#issuecomment-1396977636

   I need to look at this more closely over the weekend: I don't know why the HTTP providers have to be unique and different compared to all the others. Is the use of concepts in this PR backward? In the PR, the "free" code now _also_ "closes" resources and that feels backward to me. I expect the "close" code to also "free" resources are part of closing, not the other way around.
   Any thoughts?


-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-io] garydgregory commented on a diff in pull request #418: [IO-784] Add support for Appendable to HexDump util

Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #418:
URL: https://github.com/apache/commons-io/pull/418#discussion_r1059422260


##########
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:
   Let the caller decide when to flush and/or close.



##########
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())) {

Review Comment:
   No need for `CloseShieldOutputStream`.



-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org