Re: [boost] [hash2][review] Early review (due to holidays)

On 12/6/24 20:30, Peter Dimov via Boost wrote:

You can see practical manifestations of these examples in e.g

https://github.com/pdimov/hash2/blob/develop/include/boost/hash2/md5.hpp

where at

https://github.com/pdimov/hash2/blob/26b16023dc639ae354e246458719e4b569bc1d9...

you have

BOOST_CXX14_CONSTEXPR void transform( unsigned char const block[ 64 ] )

with the implicit precondition that `block` points at at least 64 valid bytes, and then at

https://github.com/pdimov/hash2/blob/26b16023dc639ae354e246458719e4b569bc1d9...

you have

for( int i = 0; i < 16; ++i ) { x[ i ] = detail::read32le( block + i * 4 ); }

which relies on this implicit precondition to read 16 subspans of size 4.

Assuming you had code like this: void transform( span block ) { for( int i = 0; i < 16; ++i ) { x[ i ] = detail::read32le( block.subspan(i * 4, 4) ); } } I would presume, the subspan call would have a runtime check for the offset and size. Then the converting constructor from span<const unsigned char> returned from subspan to span accepted by read32le would have to check the size again. Yes, the compiler could optimize those checks away, if it's smart enough. Or it could not, e.g. if optimizations are disabled or it fails to propagate constants for some reason. But none of these checks are actually necessary, except maybe one at the entry, to enforce the size of the entire block. Maybe not even that, if the callers of transform are guaranteed to always pass the whole blocks.