Skip to content
This repository has been archived by the owner on Mar 25, 2024. It is now read-only.

Fix FmtToIoWriter::write_str to call write_all #206

Merged
merged 1 commit into from
Aug 18, 2021

Conversation

jturner314
Copy link
Contributor

@jturner314 jturner314 commented Aug 18, 2021

I experienced bytes being silently dropped when serializing large objects to compressed files using serde_yaml and flate2/bzip2. I believe this PR will fix the issue, but I haven't tested it yet to confirm. Regardless, it's clear that this change is necessary so that FmtToIoWriter meets the contract of std::fmt::Write.

Previously, the implementation could silently fail to write some bytes. Now, the implementation should either write all the bytes or return an error.

The docs for std::fmt::Write::write_str say, "This method can only succeed if the entire string slice was successfully written, and this method will not return until all data has been written or an error occurs." So, FmtToIoWriter::write_str must call std::io::Write::write_all instead of std::io::Write::write on the inner writer in order to ensure that all the bytes are written.

Previously, the implementation could silently fail to write some
bytes. Now, the implementation should either write all the bytes or
return an error.

The docs for `std::fmt::Write::write_str` say, "This method can only
succeed if the entire string slice was successfully written, and this
method will not return until all data has been written or an error
occurs." So, `FmtToIoWriter::write_str` must call
`std::io::Write::write_all` instead of `std::io::Write::write` on the
inner writer in order to ensure that all the bytes are written.
Copy link
Owner

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thanks!

That line is from my literal first ever Rust commit.

@dtolnay dtolnay merged commit 6c06bd3 into dtolnay:master Aug 18, 2021
@jturner314 jturner314 deleted the fix-io-write branch August 18, 2021 03:53
@jturner314
Copy link
Contributor Author

Thanks for merging the fix and releasing a new version so quickly! Thanks also for writing and maintaining the serde_yaml crate. It's been really useful to me.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants