Jefffrey commented on code in PR #10444:
URL: https://github.com/apache/arrow-rs/pull/10444#discussion_r3702556932
##########
arrow-buffer/src/util/bit_util.rs:
##########
@@ -468,12 +487,16 @@ where
// 4. Shift back the result to the original position
let result_first_byte = result_first_byte << bit_offset;
- // 5. Mask the bits that are outside the relevant bits in the byte
- // so the bits until bit_offset are 1 and the rest are 0
- let mask_for_first_bit_offset = (1 << bit_offset) - 1;
+ // 5. Mask in only the bits the caller asked to process, i.e. the bits in
+ // `bit_offset..bit_offset + bits_in_this_byte`. The request may end
before the
+ // byte boundary, in which case the trailing bits must be preserved as
well.
+ //
+ // `bits_in_this_byte + bit_offset <= 8`, so the mask always fits in a
`u8`. The
+ // shift is done in `u16` only so the expression stays correct for a
full byte.
Review Comment:
would we ever mask to a full byte? i believe the assert above would protect
against that? or is this just for safety
--
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]
