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..d0d5150905a9eb05dd86569ee3dc1c9a56816ccd
--- /dev/null
+++ b/content/common/circular_memory_buffer.cc
@@ -0,0 +1,168 @@
+// 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) \
+    ((a) < (b) ? ((a) < (c) ? (a) : (c)) : ((b) < (c) ? (b) : (c)))

[tommi (sloooow) - chröme, 2013/04/02 19:34:08]

nit: std::min(a, std::min(b, c)) ?
Since you only need this in a single place I'd prefer to just inline that code
there instead of adding a macro.

[Henrik Grunell, 2013/04/03 12:47:27]

Done.
Hmm, the latest code changes I did before uploading got lost . I've re-added
them, including two more places the macro is used.

+
+namespace content {
+
+CircularMemoryBuffer::CircularMemoryBuffer(void* buffer,
+                                           size_t buffer_size)
+    : memory_buffer_(static_cast<int8*>(buffer)),
+      memory_buffer_size_(buffer_size),
+      position_(0),
+      wrap_position_(0),
+      end_position_(0),
+      total_written_(0),
+      total_read_(0) {
+  DCHECK(memory_buffer_);
+  DCHECK_GT(memory_buffer_size_, 3 * sizeof(int32));

[tommi (sloooow) - chröme, 2013/04/02 19:34:08]

document this requirement?

[Henrik Grunell, 2013/04/03 12:47:27]

Done.

+}
+
+void CircularMemoryBuffer::InitRead() {
+  GetHeaderData();
+  position_ = 3;

[tommi (sloooow) - chröme, 2013/04/02 19:34:08]

comment?

[Henrik Grunell, 2013/04/03 12:47:27]

I'm using sizeof on the new header struct to determine the header size.

+  total_read_ = 0;
+  DCHECK_LE(total_written_, memory_buffer_size_);
+  DCHECK_LT(wrap_position_, memory_buffer_size_);
+  DCHECK_LT(end_position_, memory_buffer_size_);
+}
+
+void CircularMemoryBuffer::InitWrite(size_t wrap_position) {
+  DCHECK_LT(wrap_position, memory_buffer_size_);
+  position_ = 3;

[tommi (sloooow) - chröme, 2013/04/02 19:34:08]

seems like it would be good to have a well named constant for this value.

[Henrik Grunell, 2013/04/03 12:47:27]

I'm using sizeof on the new header struct to determine the header size.

+  wrap_position_ = wrap_position;
+  SetHeaderData(0, wrap_position_, position_);
+}
+
+size_t CircularMemoryBuffer::Read(void* buffer, size_t buffer_size) {
+  if (total_read_ >= total_written_)
+    return 0;
+
+  int8* buffer_int8 = static_cast<int8*>(buffer);

[tommi (sloooow) - chröme, 2013/04/02 19:34:08]

reinterpret_cast (void and int8 aren't related types)

[Henrik Grunell, 2013/04/03 12:47:27]

Done.

+  size_t read = 0;
+
+  // Read from beginning part.
+  if (position_ < wrap_position_) {
+    size_t to_marked_pos = wrap_position_ - position_;
+    size_t to_eow = total_written_ - total_read_;
+    size_t to_read = MIN3(buffer_size, to_marked_pos, to_eow);
+    memcpy(buffer_int8 + read, memory_buffer_ + position_, to_read);
+    position_ += to_read;
+    read += to_read;
+    if (position_ == wrap_position_ &&
+        total_written_ == 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_ = 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_ >= wrap_position_);
+  if (position_ >= end_position_) {
+    size_t remaining_buffer_size = buffer_size - read;
+    size_t to_eof = memory_buffer_size_ - position_;
+    size_t to_read = remaining_buffer_size < to_eof ?

[tommi (sloooow) - chröme, 2013/04/02 19:34:08]

std::min?

[Henrik Grunell, 2013/04/03 12:47:27]

Changed to use MIN3 macro.

+        remaining_buffer_size : to_eof;
+    memcpy(buffer_int8 + read, memory_buffer_ + position_, to_read);
+    position_ += to_read;
+    read += to_read;
+    if (position_ == memory_buffer_size_) {
+      // We've read all the middle part, set position to the end part.
+      position_ = wrap_position_;
+    }
+    if (read >= buffer_size) {
+      DCHECK(read == buffer_size);
+      return read;
+    }
+  }
+
+  // Read from end part.
+  DCHECK(position_ >= wrap_position_ && position_ < end_position_);
+  size_t remaining_buffer_size = buffer_size - read;
+  size_t to_eob = end_position_ - position_;
+  size_t to_read = remaining_buffer_size < to_eob ?

[tommi (sloooow) - chröme, 2013/04/02 19:34:08]

std::min

[Henrik Grunell, 2013/04/03 12:47:27]

Changed to use MIN3 macro.

+      remaining_buffer_size : to_eob;
+  memcpy(buffer_int8 + read, memory_buffer_ + position_, to_read);
+  position_ += to_read;
+  read += to_read;
+  DCHECK(read <= buffer_size);
+  return read;
+}
+
+void CircularMemoryBuffer::Write(const void* buffer, size_t buffer_size) {
+  size_t position_before_write = position_;
+
+  size_t to_eof = memory_buffer_size_ - position_;
+  size_t to_write = buffer_size < to_eof ? buffer_size : to_eof;

[tommi (sloooow) - chröme, 2013/04/02 19:34:08]

here as well (and elsewhere if there are more)

[Henrik Grunell, 2013/04/03 12:47:27]

Done. No other places.

+  memcpy(memory_buffer_ + position_, buffer, to_write);
+  position_ += to_write;
+
+  if (position_ >= memory_buffer_size_) {
+    DCHECK_EQ(position_, memory_buffer_size_);
+    position_ = 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_ + position_,
+           static_cast<const int8*>(buffer) + to_write,

[tommi (sloooow) - chröme, 2013/04/02 19:34:08]

reinterpret_cast

[Henrik Grunell, 2013/04/03 12:47:27]

Done.

+           remainder_to_write);
+    position_ += remainder_to_write;
+  }
+
+  UpdateHeaderData(buffer_size, position_);
+}
+
+void CircularMemoryBuffer::GetHeaderData() {
+  int32* memory_buffer_int32 = reinterpret_cast<int32*>(memory_buffer_);
+
+  total_written_ = static_cast<size_t>(*memory_buffer_int32);

[tommi (sloooow) - chröme, 2013/04/02 19:34:08]

nit: what about using a struct or indices instead of incrementing?  A struct
would be nicer, but make sure that packing is not bigger than 4 bytes.
If you use indices, the code will be slightly more readable imho:
total_written_ = static_cast<size_t>(memory_buffer_int32[0]);
wrap_position_ = static_cast<size_t>(memory_buffer_int32[1]);
end_position_ = static_cast<size_t>(memory_buffer_int32[2]);

But again, I think having a struct for the header would be better.

[Henrik Grunell, 2013/04/03 12:47:27]

Using a struct now. Not sure if you wanted it only here or throughout the class.
I did the latter. I'm also using sizeof on it to determine the header size,
instead of a constant. Then, handling padding/packing/alignment shouldn't be
needed.

+  ++memory_buffer_int32;
+  wrap_position_ = static_cast<size_t>(*memory_buffer_int32);
+  ++memory_buffer_int32;
+  end_position_ = static_cast<size_t>(*memory_buffer_int32);
+}
+
+void CircularMemoryBuffer::SetHeaderData(size_t length,
+                                         size_t wrap_position,
+                                         size_t position) {
+  int32* memory_buffer_int32 = reinterpret_cast<int32*>(memory_buffer_);

[tommi (sloooow) - chröme, 2013/04/02 19:34:08]

same here and below

[Henrik Grunell, 2013/04/03 12:47:27]

Done.

+
+  *memory_buffer_int32 = static_cast<int32>(length);
+  ++memory_buffer_int32;
+  *memory_buffer_int32 = static_cast<int32>(wrap_position);
+  ++memory_buffer_int32;
+  *memory_buffer_int32 = static_cast<int32>(position);
+}
+
+void CircularMemoryBuffer::UpdateHeaderData(size_t added_length,
+                                            size_t new_position) {
+  int32* memory_buffer_int32 = reinterpret_cast<int32*>(memory_buffer_);
+
+  size_t new_length = *memory_buffer_int32 + added_length;
+  if (new_length > memory_buffer_size_)
+    new_length = memory_buffer_size_;
+  *memory_buffer_int32 = static_cast<int32>(new_length);
+  memory_buffer_int32 += 2;
+  *memory_buffer_int32 = static_cast<int32>(new_position);
+}
+
+}  // namespace content