Skip to content

block-buffer: simplify {Lazy,Eager}::split_blocks - #1218

Closed
kpp wants to merge 1 commit into
RustCrypto:masterfrom
kpp:kpp/block_buf_simpl_split_blocks
Closed

kpp wants to merge 1 commit into
RustCrypto:masterfrom
kpp:kpp/block_buf_simpl_split_blocks

Conversation

@kpp

@kpp kpp commented Sep 20, 2025

Copy link
Copy Markdown
Contributor

Use split_at so you don't need to calculate tail lenght in split_blocks fn. It reduces the amount of ptr arith, and there is no need to create tail from raw ptr, because it's created by split_at itself.

The change is no-panic:

use block_buffer::{EagerBuffer, Error, LazyBuffer, array::sizes::U8};
use no_panic::no_panic;

#[no_panic]
fn check_lazy() -> Result<(), Error> {
    let mut buffer = LazyBuffer::<U8>::try_new(&[0; 8])?;
    buffer.digest_blocks(&[0; 9], |_| {});
    Ok(())
}

#[no_panic]
fn check_eager() -> Result<(), Error> {
    let mut buffer = EagerBuffer::<U8>::try_new(&[0; 8])?;
    buffer.digest_blocks(&[0; 9], |_| {});
    Ok(())
}

fn main() {
    let _ = check_lazy();
    let _ = check_eager();
}

@newpavlov

newpavlov commented Sep 20, 2025

Copy link
Copy Markdown
Member

This change results in a potential (IIUC unreachable) panic. Compare https://rust.godbolt.org/z/jMoWd1dM6 vs https://rust.godbolt.org/z/qd3MEajPj

@kpp
kpp force-pushed the kpp/block_buf_simpl_split_blocks branch from 3d9a318 to cb6b422 Compare September 20, 2025 17:52
@kpp

kpp commented Sep 20, 2025

Copy link
Copy Markdown
Contributor Author

Fixed: https://rust.godbolt.org/z/jeMjTPMja. The new code generates no panic and it's shorter in instrs.

)
let (blocks_raw, tail) = data.split_at_unchecked(blocks_len);
let blocks = slice::from_raw_parts(blocks_raw.as_ptr().cast(), nb);
(blocks, tail)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's worth to use Array::slice_as_chunks here. It's probably worth to update the Lazy method to it as well.

@newpavlov

Copy link
Copy Markdown
Member

Closing in favor of #1220. It does not use unsafe and compiles into the same number of instructions (though it uses conditional moves instead of branches): https://rust.godbolt.org/z/43xMT3ExP

@newpavlov newpavlov closed this Sep 21, 2025
@kpp
kpp deleted the kpp/block_buf_simpl_split_blocks branch September 22, 2025 02:42
@kpp

kpp commented Sep 22, 2025

Copy link
Copy Markdown
Contributor Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants