Adding partially circular memory buffer wrapper.

content/common/circular_memory_buffer.cc

Index: content/common/circular_memory_buffer.cc
diff --git a/content/common/circular_memory_buffer.cc b/content/common/circular_memory_buffer.cc
new file mode 100644
index 0000000000000000000000000000000000000000..915d753580d6e14e61ae9440070b739acb895960
--- /dev/null
+++ b/content/common/circular_memory_buffer.cc
@@ -0,0 +1,186 @@
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/common/circular_memory_buffer.h"
+
+#include "base/logging.h"
+
+#define MIN3(a, b, c) std::min(a, std::min(b, c))

[tommi (sloooow) - chröme, 2013/04/03 14:11:32]

in the spirit of include-what-you-use I think you need #include <algorithm>
here.

[Henrik Grunell, 2013/04/05 11:21:08]

Done. Forgot to run lint before uploading. :-)

+
+namespace content {
+
+PartialCircularBuffer::PartialCircularBuffer(const uint8* buffer,
+                                             size_t buffer_size)
+    : memory_buffer_size_(buffer_size),
+      position_(0),
+      memory_buffer_read_(buffer),
+      total_read_(0),
+      memory_buffer_write_(NULL) {
+  InitRead();
+}
+
+PartialCircularBuffer::PartialCircularBuffer(uint8* buffer,
+                                             size_t buffer_size,
+                                             size_t wrap_position)
+    : memory_buffer_size_(buffer_size),
+      position_(0),
+      memory_buffer_read_(NULL),
+      total_read_(0),
+      memory_buffer_write_(buffer) {
+  InitWrite(wrap_position);
+}
+
+void PartialCircularBuffer::InitRead() {

[tommi (sloooow) - chröme, 2013/04/03 14:11:32]

Why do we need InitRead and InitWrite?  Can we do this in the relevant
constructors?

[Henrik Grunell, 2013/04/05 11:21:08]

Not really. Moved to ctors.

+  DCHECK(memory_buffer_read_);
+  DCHECK_GT(memory_buffer_size_, sizeof(header_data_));
+
+  memset(&header_data_, 0, sizeof(header_data_));
+  ReadHeaderData();
+  DCHECK_LE(header_data_.total_written, memory_buffer_size_);
+  DCHECK_LT(header_data_.wrap_position, memory_buffer_size_);
+  DCHECK_LT(header_data_.end_position, memory_buffer_size_);
+
+  position_ = sizeof(header_data_);
+  total_read_ = 0;
+}
+
+void PartialCircularBuffer::InitWrite(size_t wrap_position) {
+  DCHECK(memory_buffer_write_);
+  DCHECK_GT(memory_buffer_size_, sizeof(header_data_));
+  DCHECK_LT(wrap_position, memory_buffer_size_);
+
+  memset(&header_data_, 0, sizeof(header_data_));
+  header_data_.wrap_position = wrap_position;
+  WriteHeaderData();
+  position_ = sizeof(header_data_);
+}
+
+size_t PartialCircularBuffer::Read(void* buffer, size_t buffer_size) {
+  DCHECK(memory_buffer_read_);
+  if (total_read_ >= header_data_.total_written)
+    return 0;
+
+  uint8* buffer_uint8 = reinterpret_cast<uint8*>(buffer);
+  size_t read = 0;
+
+  // Read from beginning part.
+  if (position_ < header_data_.wrap_position) {
+    size_t to_marked_pos = header_data_.wrap_position - position_;
+    size_t to_eow = header_data_.total_written - total_read_;
+    size_t to_read = MIN3(buffer_size, to_marked_pos, to_eow);
+    memcpy(buffer_uint8 + read, memory_buffer_read_ + position_, to_read);
+    position_ += to_read;
+    total_read_ += to_read;
+    read += to_read;
+    if (position_ == header_data_.wrap_position &&
+        header_data_.total_written + sizeof(header_data_) ==
+            memory_buffer_size_) {
+      // We've read all the beginning part, set the position to the middle part.
+      // (The second condition above checks if the wrapping part is filled, i.e.
+      // writing has wrapped.)
+      position_ = header_data_.end_position;
+    }
+    if (read >= buffer_size) {
+      DCHECK(read == buffer_size);
+      return read;
+    }
+    if (read >= to_eow) {
+      DCHECK(read == to_eow);
+      return read;
+    }
+  }
+
+  // Read from middle part.
+  DCHECK(position_ >= header_data_.wrap_position);
+  if (position_ >= header_data_.end_position) {
+    size_t remaining_buffer_size = buffer_size - read;
+    size_t to_eof = memory_buffer_size_ - position_;
+    size_t to_eow = header_data_.total_written - total_read_;
+    size_t to_read = MIN3(remaining_buffer_size, to_eof, to_eow);
+    memcpy(buffer_uint8 + read, memory_buffer_read_ + position_, to_read);
+    position_ += to_read;
+    total_read_ += to_read;
+    read += to_read;
+    if (position_ == memory_buffer_size_) {
+      // We've read all the middle part, set position to the end part.
+      position_ = header_data_.wrap_position;
+    }
+    if (read >= buffer_size) {
+      DCHECK(read == buffer_size);
+      return read;
+    }
+    if (read >= to_eow) {
+      DCHECK(read == to_eow);
+      return read;
+    }
+  }
+
+  // Read from end part.
+  DCHECK(position_ >= header_data_.wrap_position &&
+         position_ < header_data_.end_position);
+  size_t remaining_buffer_size = buffer_size - read;
+  size_t to_eob = header_data_.end_position - position_;
+  size_t to_eow = header_data_.total_written - total_read_;
+  size_t to_read = MIN3(remaining_buffer_size, to_eob, to_eow);
+  memcpy(buffer_uint8 + read, memory_buffer_read_ + position_, to_read);
+  position_ += to_read;
+  total_read_ += to_read;
+  read += to_read;
+  DCHECK(read <= buffer_size);
+  DCHECK(read <= to_eow);
+  return read;
+}
+
+void PartialCircularBuffer::Write(const void* buffer, size_t buffer_size) {
+  DCHECK(memory_buffer_write_);
+  size_t position_before_write = position_;
+
+  size_t to_eof = memory_buffer_size_ - position_;
+  size_t to_write = std::min(buffer_size, to_eof);
+  memcpy(memory_buffer_write_ + position_, buffer, to_write);
+  position_ += to_write;
+
+  if (position_ >= memory_buffer_size_) {
+    DCHECK_EQ(position_, memory_buffer_size_);
+    position_ = header_data_.wrap_position;
+  }
+
+  if (to_write < buffer_size) {
+    size_t remainder_to_write = buffer_size - to_write;
+    DCHECK_LT(position_, position_before_write);
+    DCHECK_LE(position_ + remainder_to_write, position_before_write);
+    memcpy(memory_buffer_write_ + position_,
+           reinterpret_cast<const uint8*>(buffer) + to_write,
+           remainder_to_write);
+    position_ += remainder_to_write;
+  }
+
+  UpdateAndWriteHeaderData(buffer_size, position_);
+}
+
+void PartialCircularBuffer::ReadHeaderData() {
+  const HeaderData* header =
+      reinterpret_cast<const HeaderData*>(memory_buffer_read_);
+  memcpy(&header_data_, header, sizeof(header_data_));
+}
+
+void PartialCircularBuffer::WriteHeaderData() {
+  HeaderData* header = reinterpret_cast<HeaderData*>(memory_buffer_write_);
+  memcpy(header, &header_data_, sizeof(header_data_));
+}
+
+void PartialCircularBuffer::UpdateAndWriteHeaderData(size_t added_length,

[tommi (sloooow) - chröme, 2013/04/03 14:11:32]

instead of having ReadHeaderData, WriteHeaderData and UpdateAndWriteHeaderData,
what about making the struct like so:

struct BufferData {
  uint32 total_written;
  uint32 wrap_position;
  uint32 end_position;
  uint8 data[1];
};

Then use the data member to access the data and update all the fields directly
instead of via memcpy.

You'd change header_data_ to be a pointer, get rid of the memory_buffer_read_
and memory_buffer_write_ variables, |position_| would always be a zero based
index into BufferData::data and there's no need to do +sizeof(header)
calculations. 

[Henrik Grunell, 2013/04/05 11:21:08]

Indeed. Changed.
 
Changed header_data_ to buffer_data_. If you still want the buffer ptr argument
to the read ctor should be const, it would need to come in two versions. For now
I removed const.

Knowing the header size is still necessary to be able to check how much is left
of the mem buffer and not write oob. Also for some dchecks. I'm thinking if
doing:
data_size_ = memory_buffer_size_ -
      (buffer_data_->data - reinterpret_cast<uint8*>(buffer_data_));
and use data_size_ throughout the code. Then setting the packing shouldn't be
necessary. And it shouldn't really matter how large the header actually is. Or
do you think it's better to set the packing and use a constant?

+                                                     size_t new_position) {
+  HeaderData* header = reinterpret_cast<HeaderData*>(memory_buffer_write_);
+  memcpy(&header_data_, header, sizeof(header_data_));
+
+  size_t new_length = header_data_.total_written + added_length;
+  if (new_length > memory_buffer_size_)
+    new_length = memory_buffer_size_;
+  header_data_.end_position = new_position;
+
+  memcpy(header, &header_data_, sizeof(header_data_));
+}
+
+}  // namespace content