Directly coerce to DataOutputStream without using output-stream #14
Conversation
This is problematic because io/output-stream in clojure adds yet an other buffer in the middle which is uncessary since all instances in the code are OutputStream already and DataOutputStream already accepts it, secondly it creates corruption while using different streams like GZIPOutputStream for unknown reasons the BufferedOutputStream introduced by io/output-stream is preventing the last flush thus corrupting the output.
Codecov Report
@@ Coverage Diff @@
## develop #14 +/- ##
======================================
Coverage 100% 100%
======================================
Files 13 13
Lines 805 804 -1
======================================
- Hits 805 804 -1
Continue to review full report at Codecov.
|
src/clj_cbor/core.clj
Outdated
| @@ -78,15 +78,6 @@ | |||
|
|
|||
| ;; ## Encoding Functions | |||
|
|
|||
| (defn- data-output-stream | |||
There was a problem hiding this comment.
I think it'd be better to keep this function and do something similar to 760124f - if the argument is already a DataOutputStream then return it, if it is an OutputStream already then wrap it directly, otherwise use io/output-stream to coerce it before wrapping.
src/clj_cbor/core.clj
Outdated
| (encode-seq encoder output values)) | ||
| (.toByteArray buffer))) | ||
| ([encoder ^OutputStream output values] | ||
| (let [data-output (data-output-stream output)] | ||
| (let [data-output (DataOutputStream. output)] |
There was a problem hiding this comment.
This would double-wrap an existing DataOutputStream, which is probably fine in practice but not ideal.
|
@greglook Oh! that's very good point, I totally forgot the double wrap indeed, a bit stupid. So I did adapt the function and only call it where the stream comes externally. I hope I got it right this time. Thanks! |
This is problematic because io/output-stream in clojure adds yet an
other buffer in the middle which is uncessary since all instances in the
code are OutputStream already and DataOutputStream already accepts it,
secondly it creates corruption while using different streams like
GZIPOutputStream for unknown reasons the BufferedOutputStream introduced
by io/output-stream is preventing the last flush thus corrupting the
output.
To reproduce the bug try this:
The code above works if we do:
As you can see we directly provide ourself the
DataOutputStreamthus in the code above we don't call io/output-stream as explained.