Introduce MinidumpStringWriter and its test

minidump/minidump_string_writer.cc

Index: minidump/minidump_string_writer.cc
diff --git a/minidump/minidump_string_writer.cc b/minidump/minidump_string_writer.cc
new file mode 100644
index 0000000000000000000000000000000000000000..461d6acba4316c77e306949bc1035852252b3892
--- /dev/null
+++ b/minidump/minidump_string_writer.cc
@@ -0,0 +1,88 @@
+// Copyright 2014 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "minidump/minidump_string_writer.h"
+
+#include "base/logging.h"
+#include "minidump/minidump_writer_util.h"
+#include "util/numeric/safe_assignment.h"
+
+namespace crashpad {
+namespace internal {
+
+template <typename Traits>
+MinidumpStringWriter<Traits>::MinidumpStringWriter()
+    : MinidumpWritable(), string_base_(), string_() {
+}
+
+template <typename Traits>
+MinidumpStringWriter<Traits>::~MinidumpStringWriter() {
+}
+
+template <typename Traits>
+bool MinidumpStringWriter<Traits>::Freeze() {
+  DCHECK_EQ(state(), kStateMutable);
+
+  if (!MinidumpWritable::Freeze()) {

[Robert Sesek, 2014/08/01 18:27:29]

Not that it matters here, but I'd kind of expect to do local work prior to
calling the superclass implementation, otherwise you end up freezing children
before parents.

[Mark Mentovai, 2014/08/01 18:33:58]

rsesek wrote:
I think that it’s generally right to freeze the children before doing local
work, because parents might want to reach into their children to get stuff that
might not be nailed down until they’re frozen.

It doesn’t make a difference in this class, but it might in some of the more
complex ones.

[Robert Sesek, 2014/08/01 18:41:52]

Yeah, I figured it could go either way. I didn't know if there was a reasoning
for doing the children before or after.

+    return false;
+  }
+
+  size_t string_bytes = string_.size() * sizeof(string_[0]);
+  if (!AssignIfInRange(&string_base_.Length, string_bytes)) {
+    LOG(ERROR) << "string_bytes " << string_bytes << " out of range";
+    return false;
+  }
+
+  return true;
+}
+
+template <typename Traits>
+size_t MinidumpStringWriter<Traits>::SizeOfObject() {
+  DCHECK_GE(state(), kStateFrozen);
+
+  // Include the NUL terminator.
+  return sizeof(string_base_) + (string_.size() + 1) * sizeof(string_[0]);
+}
+
+template <typename Traits>
+bool MinidumpStringWriter<Traits>::WriteObject(
+    FileWriterInterface* file_writer) {
+  DCHECK_EQ(state(), kStateWritable);
+
+  WritableIoVec iov;

[Robert Sesek, 2014/08/01 18:27:29]

I stared at this for a while and couldn't figure out why you write both the
string_base_ and string_ to the iovec.

+  iov.iov_base = &string_base_;
+  iov.iov_len = sizeof(string_base_);
+  std::vector<WritableIoVec> iovecs(1, iov);
+
+  // Include the NUL terminator.
+  iov.iov_base = &string_[0];
+  iov.iov_len = (string_.size() + 1) * sizeof(string_[0]);
+  iovecs.push_back(iov);
+
+  return file_writer->WriteIoVec(&iovecs);
+}
+
+// Explicit template instantiation of the forms of MinidumpStringWriter<> used
+// as base classes.
+template class MinidumpStringWriter<MinidumpStringWriterUTF16Traits>;
+template class MinidumpStringWriter<MinidumpStringWriterUTF8Traits>;
+
+void MinidumpUTF16StringWriter::SetUTF8(const std::string& string_utf8) {
+  DCHECK_EQ(state(), kStateMutable);
+
+  set_string(MinidumpWriterUtil::ConvertUTF8ToUTF16(string_utf8));
+}
+
+}  // namespace internal
+}  // namespace crashpad