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

From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2024-12-06 17:25:52

On 12/6/24 20:12, Peter Dimov via Boost wrote:
> Andrey Semashev wrote:
>> And fixed extents are not as useful as the dynamic extent in general, in my
>> experience, as most of the time we deal with variable-sized sequences.
> 
> I also used to think that way, but that was because I didn't understand
> the purpose of span.
> 
> The purpose of span is to replace pointer arguments. If your function
> takes
> 
>   void f1( unsigned char p[] );
> 
> you use
> 
>   void f1( span<unsigned char> p );
> 
> and if it takes
> 
>   void f2( unsigned char p[4] );
> 
> you use
> 
>   void f2( span<unsigned char, 4> p );
> 
> The fixed extent span here does two things: first, it inserts a runtime
> check that the extent of the passed span is >= 4.
> 
> void g( span<unsigned char> p )
> {
>     f2( p ); // implicit assert( p.size() >= 4 );
> }
> 
> Second, it can be used to optimize out runtime checks in operator[]:
> 
> void f2( span<unsigned char, 4> p )
> {
>     uint32_t v = p[0] + (p[1] << 8) + (p[2] << 16) + (p[3] << 24);
> }
> 
> Since it's statically known that 0<4, 1<4, 2<4, 3<4, these accesses don't
> cause any asserts to be inserted.

Yes, I understand, but the thing is I very rarely have to write `void
f2( unsigned char p[4] );` in the first place. Most of the time I get a
variable amount of data that I need to process, so I have either an
iterator range or a pointer and size. And if there are fixed-sized
fragments of that data that I need to process, pretty much always I have
checked the entire size (or at least some outer size) of the data
beforehand, so no checks needed for those individual fragments.

So, for example, if I have to parse an RTP packet, I would

void on_rtp_packet(const uint8_t* packet, size_t size)
{
  // RTP fixed header is 12 bytes long
  if (size < 12)
    throw std::invalid_argument("RTP packet too short");

  // Parse 12-byte fixed header
  uint16_t seqn = read_be16(packet + 2);
  uint32_t timestamp = read_be32(packet + 4);
}

Here, I may accept an iterator_range<const uint8_t*> or span<const
uint8_t> as argument in on_rtp_packet, it's not important. The essential
part is that read_be* don't need to check the size because the caller
already has.