tustvold commented on code in PR #2231:
URL: https://github.com/apache/arrow-rs/pull/2231#discussion_r933992495
##########
parquet/src/util/bit_util.rs:
##########
@@ -330,103 +278,71 @@ impl BitWriter {
/// Writes the `num_bits` LSB of value `v` to the internal buffer of this
writer.
/// The `num_bits` must not be greater than 64. This is bit packed.
- ///
- /// Returns false if there's not enough room left. True otherwise.
#[inline]
- pub fn put_value(&mut self, v: u64, num_bits: usize) -> bool {
+ pub fn put_value(&mut self, v: u64, num_bits: usize) {
assert!(num_bits <= 64);
+ let num_bits = num_bits as u8;
assert_eq!(v.checked_shr(num_bits as u32).unwrap_or(0), 0); // covers
case v >> 64
- if self.byte_offset * 8 + self.bit_offset + num_bits > self.max_bytes
as usize * 8
- {
- return false;
- }
-
+ // Add value to buffered_values
self.buffered_values |= v << self.bit_offset;
self.bit_offset += num_bits;
- if self.bit_offset >= 64 {
- memcpy_value(
- &self.buffered_values,
- 8,
- &mut self.buffer[self.byte_offset..],
- );
- self.byte_offset += 8;
- self.bit_offset -= 64;
- self.buffered_values = 0;
+ if let Some(remaining) = self.bit_offset.checked_sub(64) {
+ self.buffer
+ .extend_from_slice(&self.buffered_values.to_le_bytes());
Review Comment:
I agree, they should all use to_le_bytes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]