From 901517bc0188860e6ad08b4a2113114d77541b36 Mon Sep 17 00:00:00 2001 From: Josh Yelon Date: Thu, 14 Oct 2021 14:41:03 -0400 Subject: [PATCH] Fix some of the comments on StreamBuffer --- luprex/core/cpp/streambuffer.cpp | 8 +++- luprex/core/cpp/streambuffer.hpp | 64 ++++++++++++++++++-------------- 2 files changed, 43 insertions(+), 29 deletions(-) diff --git a/luprex/core/cpp/streambuffer.cpp b/luprex/core/cpp/streambuffer.cpp index 14683951..3427040c 100644 --- a/luprex/core/cpp/streambuffer.cpp +++ b/luprex/core/cpp/streambuffer.cpp @@ -89,6 +89,13 @@ void StreamBuffer::make_space_slow(int64_t bytes) { write_cursor_ = buf_lo_ + data_size; } +void StreamBuffer::wrote_space(int64_t bytes) { + int64_t available = buf_hi_ - write_cursor_; + assert(bytes >= 0); + assert(available >= bytes); + write_cursor_ += bytes; +} + char *StreamBuffer::get_overwrite(int64_t size, int64_t write_count_after) { int64_t write_count_before = write_count_after - size; assert(write_count_before >= total_reads()); @@ -159,7 +166,6 @@ static inline bool safe_to_cast_to_uint64(uint64_t vv) { return true; } - void StreamBuffer::write_bytes(const char *s, int64_t len) { make_space(len); memcpy(write_cursor_, s, len); diff --git a/luprex/core/cpp/streambuffer.hpp b/luprex/core/cpp/streambuffer.hpp index 8eef118c..cdd0644a 100644 --- a/luprex/core/cpp/streambuffer.hpp +++ b/luprex/core/cpp/streambuffer.hpp @@ -17,7 +17,7 @@ // const int bufsize = 16384; // // // Allocate transient space in the streambuffer. -// char *space = streambuffer.alloc_space(bufsize); +// char *space = streambuffer.make_space(bufsize); // // // Call the linux 'read' function. // ssize_t bytes_read = read(fd, space, bufsize); @@ -25,17 +25,17 @@ // // Append the bytes read to the streambuffer. // streambuffer.wrote_space(bytes_read); // -// Now, let's dig into the semantics of this. The method 'alloc_space' MUST be -// followed by 'wrote_space'. It is an error to invoke these methods unless you -// do them in that sequence. Together, these two methods count as a single -// 'write' operation into the StreamBuffer. +// The make_space operation allocates an array of bytes where the data can be +// written, and returns a pointer to that array of bytes. The read operation +// fills some or all of the allocated bytes. Finally, the wrote_space operation +// notifies the StreamBuffer that some of the bytes have been filled with data. +// These bytes are appended to the StreamBuffer. // -// 'alloc_space' allocates a block of bytes within the StreamBuffer. The -// pointer returned here is only valid until the 'wrote_space' operation. The -// method 'wrote_space' tells the StreamBuffer that the space has been populated -// with the specified amount of data. The data is then officially appended to -// the StreamBuffer. Again, the two methods 'alloc_space' followed by -// 'wrote_space' together count as a single write operation. +// The pointer returned by 'make_space' is only valid until you mutate the +// StreamBuffer. Therefore, you should call 'make_space', then immediately fill +// the bytes. It is imperative that 'wrote_space' be the first mutator after +// 'make_space.' You should think of 'make_space' followed by 'wrote_space' as +// a single two-phase operation. // // THE OVERWRITE_INT METHODS: // @@ -101,21 +101,6 @@ // When you call read_bytes, it returns a pointer to a block of bytes. This // pointer only remains valid until you do a 'write' into the stream. // -// NESTED DECODING -// -// Here is an interesting construct: -// -// // Read a message from the stream. -// size_t len = streambuffer.read_int32() -// const char *bytes = streambuffer.read_bytes(len); -// -// // Construct another stream object to decode the message. -// StreamBuffer substream(bytes, len); -// decode(substream); -// -// This is perfectly valid and a potentially convenient way to parse the -// contents of a message. -// // UNREADING BYTES // // It's possible to 'unread' bytes that you've already read from a stream. This @@ -167,6 +152,23 @@ // total_writes for such a buffer returns the 'len' value that you initialized // the buffer with. // +// NESTED DECODING +// +// Here is an interesting construct: +// +// // Read a message from the stream. +// size_t len = streambuffer.read_int32() +// const char *bytes = streambuffer.read_bytes(len); +// +// // Construct another stream object to decode the message. +// StreamBuffer substream(bytes, len); +// decode(substream); +// +// This is perfectly valid and a potentially convenient way to parse the +// contents of a message. Note that the substream contains a pointer to +// the parent stream's buffer, and therefore, data corruption will occur +// if you mutate the parent stream while reading the substream. +// // USING A STREAMBUFFER TO READ AN ENTIRE FILE // // If you wish to read an entire file and store the file contents in a @@ -281,7 +283,9 @@ public: // Read a block of bytes from the buffer. // - // Throws StreamEof if the specified number of bytes aren't present. + // Caution: the pointer returned is a pointer to the stream's buffer. It is + // only valid until you mutate the buffer. Throws StreamEof if the specified + // number of bytes aren't present. // const char *read_bytes(int64_t bytes); @@ -409,12 +413,16 @@ private: void init(bool fixed, bool owned, char *buf, int64_t size); // Make the specified amount of space in the buffer for writing. - void make_space(int64_t bytes) { + // Return a pointer to the space. + char *make_space(int64_t bytes) { int64_t available = buf_hi_ - write_cursor_; if (available < bytes) make_space_slow(bytes); + return write_cursor_; } void make_space_slow(int64_t bytes); + void wrote_space(int64_t bytes); + // Make sure the specified number of bytes are available to read. void check_available(int64_t bytes) { int64_t avail = write_cursor_ - read_cursor_;