Repository navigation
Keep the input in the result of Zstd.write_skippable_frame - #163
Open
sribalakumar wants to merge 1 commit into
Open
sribalakumar wants to merge 1 commit into
sribalakumar wants to merge 1 commit into
Conversation
write_skippable_frame(input, data) allocated room for the input plus the skippable frame, but returned only the frame: ZSTD_writeSkippableFrame writes the frame from the start of the buffer and the result was then truncated to the frame's size. The input was discarded, so the README example's compressed_data_with_skippable_frame no longer contained the compressed data, and Zstd.decompress on it raised "not a zstd frame". Write the frame and then the input after it. A leading frame is what read_skippable_frame expects, and Zstd.decompress skips skippable frames to reach the data that follows.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #161.
Change: write the skippable frame, then copy the input after it, and resize to frame size + input size. Also adds
RB_GC_GUARDfor both string arguments.New specs (4 examples) in
spec/zstd-skippable_frame_spec.rb, underdescribe 'write_skippable_frame':Zstd.decompressmagic_variant(magic_variant: 15, checks bothread_skippable_frameandZstd.decompress)Before
Current main,
rspec spec/zstd-skippable_frame_spec.rb: 15 examples, 3 failures.("accepts an empty input" passes before too, since there's no payload to lose.)
After
15 examples, 0 failures. Full suite: 95 examples, 0 failures.
The README example now works end to end:
read_skippable_framereturns"sample data"andZstd.decompressreturns the original data.Compatibility note
Anyone who worked around the old behaviour by concatenating the frame and the data themselves would now get the data twice, and
Zstd.decompresswould return the payload twice. If you'd rather not change behaviour, the alternative is to keep it as-is and correct the README instead — happy to switch to that approach if you'd prefer.Separate thing noticed while testing (not fixed here)
While testing this, I noticed
Zstd::StreamingDecompress#decompressreturns an empty string when its input begins with a skippable frame — or an empty frame such asZstd.compress("")— followed by real data; its loop stops at the first frame that produces no output. This is independent of this PR and also happens with a hand-built skippable frame, andZstd.decompressandZstd::StreamReaderboth handle it fine. Happy to open a separate issue for that if useful.