Adds write_pod() to tiered storage's ByteBlockWriter
#34409
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #34409 +/- ##
=========================================
- Coverage 81.9% 81.8% -0.1%
=========================================
Files 819 819
Lines 220984 220989 +5
=========================================
- Hits 181015 180971 -44
- Misses 39969 40018 +49 |
| /// Write the specified typed instance to the internal buffer of | ||
| /// the ByteBlockWriter instance. | ||
| /// | ||
| /// Prefer `write_pod()` when possible, because `write_type()` may cause | ||
| /// undefined behavior if `value` contains uninitized bytes. |
There was a problem hiding this comment.
Thanks for also improving the comment. Do you think we should deprecate write_type? Or we just make write_pod write_type?
There was a problem hiding this comment.
We still need write_type() for some situations, so cannot remove it entirely.
However, I do not intend to merge this PR, as I've since put up #34415, which handles these concerns better.
There was a problem hiding this comment.
Got it. Let me convert it to a draft. Feel free to convert it back when you think it's needed.
There was a problem hiding this comment.
This PR is ready for review, and is not a draft. But I'd rather solve this problem by #34415 instead. Please let's review that one first. If that one is merged, I'll close this PR.
|
Closing. This problem has been resolved by #34415. |
Problem
In Tiered Storage's ByteBlockWriter, it is possible to invoke undefined behavior with
write_type(), if called with a value that contains uninitialized bytes. This happens with structs that contain implicit padding, e.g.HotAccountMeta.As an incremental improvement, if we know when have a value with no uninitialized bytes, we can ensure no undefined behavior by using
bytemuck::bytes_from().Summary of Changes
Leverage bytemuck to provide an undefined-behavior-free version of
write_type(),write_pod().