Adding partially circular memory buffer wrapper.

content/common/circular_memory_buffer.h

+// 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.
+
+#ifndef CONTENT_COMMON_CIRCULAR_MEMORY_BUFFER_H_
+#define CONTENT_COMMON_CIRCULAR_MEMORY_BUFFER_H_
+
+#include "base/basictypes.h"
+
+namespace content {
+
+// A wrapper around a memory buffer that allows circular read and write with a
+// selectable wrapping position. Buffer layout (after wrap; H is header):
+// -----------------------------------------------------------
+// | H | Beginning           | End           | Middle        |
+// -----------------------------------------------------------
+//  ^---- Non-wrapping -----^ ^--------- Wrapping ----------^
+// The non-wrapping part is never overwritten. The wrapping part will be
+// circular. The first 3 * 4 bytes (int32) = 12 bytes + possible padding is the

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

see comment about this below.  As is sizeof(header) on 64bit platforms would be
3*8==24 bytes.  On 64bit platforms using uint32 instead of size_t it would be 16
bytes if you don't specify how the packing should be done.

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

Absolutely, I had forgot to update this after those changes. I've however now
changed to the struct pointer approach (without packing; see other comment) and
then I think it be better not to discuss hard numbers here at all. The point is
that there's a small header, and the buffer size must be larger than that. For
normal cases the buffer will probably be much larger, since the purpose of the
class is to deal with pretty large amounts of data. What do you think?

+// header. It consists of the following information:
+// - Length written to the buffer (not including header).
+// - Wrapping position.
+// - End position of buffer. (If the last byte is at x, this will be x + 1.)
+// Users of wrappers around the same underlying buffer must ensure that writing
+// is finished before reading is started.
+class PartialCircularBuffer {
+ public:
+  // Use for reading. |buffer_size| is in bytes and must be larger than the
+  // header size (see above).
+  PartialCircularBuffer(const uint8* buffer, size_t buffer_size);
+
+  // Use for writing. |buffer_size| is in bytes and must be larger than the
+  // header size (see above).
+  PartialCircularBuffer(uint8* buffer,
+                        size_t buffer_size,
+                        size_t wrap_position);
+
+  size_t Read(void* buffer, size_t buffer_size);
+  void Write(const void* buffer, size_t buffer_size);
+
+ private:
+  struct HeaderData {

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

I don't think it's a good idea to use size_t here since size_t isn't always the
same size.  Before you were using uint32 and I think that's appropriate here.

I also think that we should force packing to be 4 (sizeof(uint32)) here as well.
 If we don't do that then the size of the struct on 64bit platforms cold be
4*sizeof(uint32) and not 3*sizeof(uint32).

See #pragma pack elsewhere in the code for how to do this.

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

Agree. Changed to uint32.

I can still set the packing if you think, but not sure if needed. See my other
comment. Not sure either how much the penalty would be to pack it, i.e. if
there's a good reason not to pack (given that other potential problems are
avoided which I think they are).

+    size_t total_written;
+    size_t wrap_position;
+    size_t end_position;
+  };
+
+  void InitRead();
+  void InitWrite(size_t wrap_position);
+
+  void ReadHeaderData();
+  void WriteHeaderData();
+  void UpdateAndWriteHeaderData(size_t added_length, size_t new_position);
+
+  // Used for reading and writing.
+  size_t memory_buffer_size_;
+  size_t position_;
+  HeaderData header_data_;

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

maybe you should keep this variable last in the class to make life easier for
the compiler (and for us to read the crash dumps).  That way padding will be
added to the end of the class instead of between this variable and
memory_buffer_read_ .

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

It's now a pointer.

+
+  // Used for reading.
+  const uint8* memory_buffer_read_;
+  size_t total_read_;
+
+  // Used for writing.
+  uint8* memory_buffer_write_;
+
+};
+
+}  // namespace content
+
+#endif  // CONTENT_COMMON_CIRCULAR_MEMORY_BUFFER_H_