Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zstd: free Decoder resources when Reset is called with a nil io.Reader #305

Merged
merged 2 commits into from
Dec 30, 2020

Conversation

mostynb
Copy link
Contributor

@mostynb mostynb commented Dec 25, 2020

Attempt at fixing #296.

@mostynb
Copy link
Contributor Author

mostynb commented Dec 26, 2020

(I will dig into the test failures in the next few days.)

Copy link
Owner

@klauspost klauspost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution.

Let's update the docs as well.

// Reset will reset the decoder the supplied stream after the current has finished processing.
// Note that this functionality cannot be used after Close has been called.
// Reset can be called with a nil reader to release references to the previous reader.
// When sending a nil reader no other operations than Reset or DecodeAll or Close should be used.
func (d *Decoder) Reset(r io.Reader) error {

A test that also tests the basic functionality, ie create a Reader, Reset with nil and check that reads returns the expected error and finally that it can be used with a proper reader would be great.

zstd/decoder.go Outdated Show resolved Hide resolved
zstd/decoder.go Outdated Show resolved Hide resolved
…o.Reader

* Remove unneeded alloc.
* Update docs.
* Return a sentinel error when trying to read from a nil reader.
* Return the same error if the Decoder was created with a nil reader.
@klauspost
Copy link
Owner

Thanks. Looks good. I will give it a final look through and merge when I have the time.

@klauspost klauspost merged commit fa5ea64 into klauspost:master Dec 30, 2020
@mostynb mostynb deleted the decoder_reset_nil branch December 30, 2020 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants