Skip to content

Commit

Permalink
Make prepend_alloc try harder
Browse files Browse the repository at this point in the history
Previously prepend_alloc would only try to allocate headspace if the
empty space at the beginning of the buffer was available. This change
maintains this as the preferred behavior but also allows for the
possibility of moving existing data down the buffer if this will make
the required space available.

This change is motivated by the fact that the current behavior can lead
to an exception being thrown if there are plenty of bytes available at
the end of the buffer but not at the beginning. This can happen if
.realign(0) is called on a buffer that will later have data prepended to
it.

Signed-off-by: Charlie Vigue <[email protected]>
  • Loading branch information
cvigue committed Jan 9, 2025
1 parent ea06ff5 commit a96c32c
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 9 deletions.
20 changes: 11 additions & 9 deletions openvpn/buffer/buffer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,9 @@ class ConstBufferType
* @brief Allocate space for prepending data to the buffer.
* @param size The number of T objects to allocate space for.
* @return A pointer to the allocated space in the buffer.
* @note This function may move the data in the buffer to make room for the prepended
* data. If insufficient space is available, this will throw with the strong
* exception guarantee.
*/
T *prepend_alloc(const size_t size);

Expand Down Expand Up @@ -1574,16 +1577,15 @@ T *ConstBufferType<T>::write_alloc(const size_t size)
}

template <typename T>
T *ConstBufferType<T>::prepend_alloc(const size_t size)
T *ConstBufferType<T>::prepend_alloc(const size_t request_size)
{
if (size <= offset_)
{
offset_ -= size;
size_ += size;
return data();
}
else
OPENVPN_BUFFER_THROW(buffer_headroom);
if (request_size > offset())
realign(request_size);

offset_ -= request_size;
size_ += request_size;

return data();
}

template <typename T>
Expand Down
63 changes: 63 additions & 0 deletions test/unittests/test_buffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,69 @@ TEST(buffer, alloc_buffer_read1)
EXPECT_EQ(memcmp(raw, data, sizeof(raw)), 0);
}

TEST(buffer, prepend_alloc)
{
BufferAllocated buf(64, 0);
buf_append_string(buf, "hello world");
EXPECT_EQ(buf.offset(), 0u);

buf.prepend_alloc(5);
EXPECT_EQ(buf.size(), 16u);
EXPECT_EQ(buf.remaining(), 48u);
}


TEST(buffer, prepend_alloc_2)
{
BufferAllocated buf(64, 0);
EXPECT_EQ(buf.offset(), 0u);
buf.init_headroom(2);
EXPECT_EQ(buf.offset(), 2u);
buf_append_string(buf, "hello world");
EXPECT_EQ(buf.offset(), 2u);

buf.prepend_alloc(5);
EXPECT_EQ(buf.offset(), 0u);
EXPECT_EQ(buf.size(), 16u);
EXPECT_EQ(buf.remaining(), 48u);
}


TEST(buffer, prepend_alloc_fits)
{
BufferAllocated buf(64, 0);
EXPECT_EQ(buf.offset(), 0u);
buf.init_headroom(5);
EXPECT_EQ(buf.offset(), 5u);
buf_append_string(buf, "hello world");
EXPECT_EQ(buf.offset(), 5u);

buf.prepend_alloc(5);
EXPECT_EQ(buf.offset(), 0u);
EXPECT_EQ(buf.size(), 16u);
EXPECT_EQ(buf.remaining(), 48u);
}

TEST(buffer, prepend_alloc_fail)
{
BufferAllocated buf(11, 0);
buf_append_string(buf, "hello world");

EXPECT_THROW(buf.prepend_alloc(5), std::exception);
EXPECT_EQ(buf.size(), 11u);
EXPECT_EQ(buf.remaining(), 0u);
}

TEST(buffer, prepend_alloc_fail2)
{
BufferAllocated buf(14, 0);
buf_append_string(buf, "hello world");

EXPECT_THROW(buf.prepend_alloc(5), std::exception);
EXPECT_EQ(buf.size(), 11u);
EXPECT_EQ(buf.remaining(), 3u);
}

TEST(buffer, realign)
{
BufferAllocated buf(64, 0);
Expand Down

0 comments on commit a96c32c

Please sign in to comment.