-
Notifications
You must be signed in to change notification settings - Fork 119
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
base64ct: minor code improvements #234
Conversation
|
||
let flag = src_rem.len() == 1; | ||
let mask = (flag as u8).wrapping_sub(1); | ||
dst_rem[2] = (dst_rem[2] & mask) | (PAD & !mask); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also note that I have replaced the explicit branch with this bit-twiddling. I wonder if there is a better way to generate mask from bool
.
fn encode_string(input: &[u8], padded: bool, hi_bytes: (u8, u8)) -> String { | ||
let elen = encoded_len(input, padded); | ||
let elen = encoded_len_inner(input.len(), padded).expect("input is too big"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks to the forced inlining, the second expect
used on encode
result will be properly removed. This change allows this function to panic before allocation, which simplifies generated assembly a bit.
} else { | ||
encode_3bytes_padded(s, d, hi_bytes); | ||
} | ||
if let Some(dst_rem) = dst_chunks.next() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh neat, I guess that was all I was missing.
Improves the
encode
function by improving code reuse and removing an unreachable panic in the padded path. Also marks all inner functions with#[inline(always)]
to remove call indirection. Most often crates will use only one module, so I think such inlining is warranted.