commit 53020a5d3361adca1c36b6a79221858f5954c3c1
author bnc <bnc@google.com> Mon May 16 12:35:34 2022 -0700
committer Copybara-Service <copybara-worker@google.com> Mon May 16 12:37:09 2022 -0700
tree e362f9ec449535f650f64727768f4ebaacdae7f7
parent 73fe5e2fe464313f2e5bd0187d00184e1de4c3a8

Lazy reserve in SimpleBuffer.

Also make sure at least kMinimumSimpleBufferSize bytes are allocated, even if
constructor with size argument is called with a smaller value.

This slightly simplifies the implementation, allows removal of QUICH_CHECK in
SimpleBuffer::SimpleBuffer(int size) (since there is a QUICHE_BUG in Reserve(),
and QUICHE_BUG is more suitable for API violations than a CHECK anyway), and
makes it more obvious in Reserve() that `storage_` may be nullptr.

The issue that prompted this change is that SimpleBufferTest.ReleaseAsSlice
crashes at the memcpy on old line 104 on Envoy CI with ASAN build (which implies
UBSAN).  This is because ReleaseAsSlice() leaves SimpleBuffer (unless it was
empy beforehand) in an unusual state of `storage_` being nullptr, on the
reasonable assumption that the object will probably not be used again.  But it
is also reasonable to assume that the object is left in a valid state, so that
it can be read from and written to, and Reserve() will take care of allocating
storage if necessary.  Except Reserve() calls memcpy with nullptr (and zero
count) in this case.

And memcpy must not be called with nullptr, even if count is zero, otherwise
behavior is undefined according to
https://en.cppreference.com/w/cpp/string/byte/memcpy.

Instead of applying a Band-aid of special casing `storage_` being nullptr before
the memcpy call, and relying on test coverage only in tests that call
ReleaseAsSlice() to put SimpleBuffer in this special case, it is more elegant to
make this the default state of SimpleBuffer, so that it is handled more
prominently in the implementation, and tested more thoroughly.

The cost is an extra method call and a few branches when the object is first
used.  The gain is one allocation saved if the initial write is larger than 10
bytes, and one allocation saved if the object is never used.

Also make Reserve() call unconditional in Write() for simplicity.  Reserve()
makes the same check anyway.

Also get rid of unnecessary `new_storage_size` local variable, and use
std::max() instead of branch.

Also remove Test prefix from test cases.

PiperOrigin-RevId: 449027799