Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions source/common/common/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
#include <chrono>
#include <cmath>
#include <cstdint>
#include <ios>
#include <iostream>
#include <iterator>
#include <regex>
#include <string>
Expand Down Expand Up @@ -221,6 +223,15 @@ std::string DateFormatter::now(TimeSource& time_source) {
return fromTime(time_source.systemTime());
}

FixedSizeStreamBuffer::FixedSizeStreamBuffer(char* base, size_t size) {
this->setp(base, base + size);
}

OutputBufferStream::OutputBufferStream(char* data, size_t size)
: FixedSizeStreamBuffer{data, size}, std::ostream{static_cast<std::streambuf*>(this)} {}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does the call to setp need to happen before a pointer is given to the ostream constructor?

This seems to work:
/**
* std::ostream class that serializes writes into the provided buffer.
*/
class OutputBufferStream : private std::streambuf, public std::ostream {
public:
OutputBufferStream(char* data, size_t size);
...
}

OutputBufferStream::OutputBufferStream(char* data, size_t size)
: std::ostream(this) {
setp(data, data + size);
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The call to setp doesn't need to occur first since streambuf are more focused on managing some storage (buffer, file, ...) and ostreams are more focused on serializing objects. ostream defers the actual writing / storage management to the streambuf.

I organized it like this to be consistent with the InputConstMemoryStream. If you want I can flatten it (removing FixedSizeStreamBuffer)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would consider using the name MutableMemoryStreamBuffer

I don't feel strongly about flattening it. Flattening it seems slightly cleaner but both approaches are fine.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.


int OutputBufferStream::numValidBytesWrittenToBuffer() { return pptr() - pbase(); }

ConstMemoryStreamBuffer::ConstMemoryStreamBuffer(const char* data, size_t size) {
// std::streambuf won't modify `data`, but the interface still requires a char* for convenience,
// so we need to const_cast.
Expand Down
22 changes: 22 additions & 0 deletions source/common/common/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <chrono>
#include <cstdint>
#include <ios>
#include <set>
#include <sstream>
#include <string>
Expand Down Expand Up @@ -110,6 +111,27 @@ class RealTimeSource : public TimeSource {
MonotonicTime monotonicTime() override { return std::chrono::steady_clock::now(); }
};

/**
* Class used for creating non-memory allocating std::ostream.
*/
class FixedSizeStreamBuffer : public std::streambuf {
Comment thread
KBaichoo marked this conversation as resolved.
Outdated
public:
FixedSizeStreamBuffer(char* base, size_t size);
};

/**
* std::ostream class that serializes writes into the provided buffer.
*/
class OutputBufferStream : private FixedSizeStreamBuffer, public std::ostream {
public:
OutputBufferStream(char* data, size_t size);

/**
* @return the number of bytes written prior to the "put" pointer into the buffer.
*/
int numValidBytesWrittenToBuffer();
Comment thread
KBaichoo marked this conversation as resolved.
Outdated
};

/**
* Class used for creating non-copying std::istream's. See InputConstMemoryStream below.
*/
Expand Down
1 change: 1 addition & 0 deletions test/common/common/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ envoy_cc_test(
],
deps = [
"//source/common/common:utility_lib",
"//test/common/stats:stat_test_utility_lib",
"//test/test_common:simulated_time_system_lib",
"//test/test_common:test_time_lib",
"//test/test_common:utility_lib",
Expand Down
63 changes: 63 additions & 0 deletions test/common/common/utility_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include "common/common/utility.h"

#include "test/common/stats/stat_test_utility.h"
#include "test/test_common/simulated_time_system.h"
#include "test/test_common/test_time.h"
#include "test/test_common/utility.h"
Expand Down Expand Up @@ -116,6 +117,68 @@ TEST(DateUtil, NowToMilliseconds) {
EXPECT_EQ(12345067, DateUtil::nowToMilliseconds(test_time));
}

TEST(OutputBufferStream, FailsOnWriteToEmptyBuffer) {
constexpr char data = 'x';
OutputBufferStream ostream{nullptr, 0};
ASSERT_TRUE(ostream.good());

ostream << data;

EXPECT_TRUE(ostream.bad());
}

TEST(OutputBufferStream, CanWriteToBuffer) {
constexpr char data[] = "123";
std::string buffer;
buffer.resize(3);

OutputBufferStream ostream{buffer.data(), buffer.size()};
ostream << data;

EXPECT_EQ(buffer, data);
}

TEST(OutputBufferStream, CannotOverwriteBuffer) {
constexpr char data[] = "123";
std::string buffer;
buffer.resize(2);

OutputBufferStream ostream{buffer.data(), buffer.size()};
ostream << data << std::endl;

EXPECT_EQ(buffer, "12");
}

TEST(OutputBufferStream, DoesNotAllocateMemoryEvenIfWeTryToOverflowBuffer) {
constexpr char data[] = "123";
std::string buffer;
buffer.resize(2);
Stats::TestUtil::MemoryTest memory_test;

OutputBufferStream ostream{buffer.data(), buffer.size()};
ostream << data << std::endl;

EXPECT_EQ(buffer, "12");
EXPECT_EQ(memory_test.consumedBytes(), 0);
}

TEST(OutputBufferStream, ReturnsNumberOfBytesWrittenIntoBuffer) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This test seems very similar to OutputBufferStream.CanWriteToBuffer

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Folded into that test and the overflow test.

constexpr char data[] = "123";
std::string buffer;
buffer.resize(3);
Comment thread
KBaichoo marked this conversation as resolved.
Outdated

OutputBufferStream ostream{buffer.data(), buffer.size()};
ASSERT_EQ(ostream.numValidBytesWrittenToBuffer(), 0);

ostream << data;
ASSERT_EQ(buffer, data);
EXPECT_EQ(ostream.numValidBytesWrittenToBuffer(), 3);

// Try to overflow buffer, should still have 3 as bytes written.
ostream << data;
EXPECT_EQ(ostream.numValidBytesWrittenToBuffer(), 3);
}

TEST(InputConstMemoryStream, All) {
{
InputConstMemoryStream istream{nullptr, 0};
Expand Down