chromeos: Make dbus::MessageReader memory ownership explicit

Make memory returned by MessageReader::PopArrayOfBytes()
const to make it clearer that ownership remains with the
MessageReader.

Also update PopArrayOfStrings() and PopArrayOfObjectPaths()
to clear the passed-in vectors before appending to them.

BUG=none
TBR=isherman@chromium.org,mvanouwerkerk@chromium.org

Review URL: https://codereview.chromium.org/176693003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@252922 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 20 insertions, 14 deletions

diff --git a/dbus/message.cc b/dbus/message.cc
index 918b860..eaf3c9b 100644
--- a/dbus/message.cc
+++ b/dbus/message.cc
@@ -807,7 +807,7 @@ bool MessageReader::PopVariant(MessageReader* sub_reader) {
   return PopContainer(DBUS_TYPE_VARIANT, sub_reader);
 }
 
-bool MessageReader::PopArrayOfBytes(uint8** bytes, size_t* length) {
+bool MessageReader::PopArrayOfBytes(const uint8** bytes, size_t* length) {
   MessageReader array_reader(message_);
   if (!PopArray(&array_reader))
       return false;
@@ -829,9 +829,10 @@ bool MessageReader::PopArrayOfBytes(uint8** bytes, size_t* length) {
 
 bool MessageReader::PopArrayOfStrings(
     std::vector<std::string> *strings) {
+  strings->clear();
   MessageReader array_reader(message_);
   if (!PopArray(&array_reader))
-      return false;
+    return false;
   while (array_reader.HasMoreData()) {
     std::string string;
     if (!array_reader.PopString(&string))
@@ -843,9 +844,10 @@ bool MessageReader::PopArrayOfStrings(
 
 bool MessageReader::PopArrayOfObjectPaths(
     std::vector<ObjectPath> *object_paths) {
+  object_paths->clear();
   MessageReader array_reader(message_);
   if (!PopArray(&array_reader))
-      return false;
+    return false;
   while (array_reader.HasMoreData()) {
     ObjectPath object_path;
     if (!array_reader.PopObjectPath(&object_path))
@@ -858,9 +860,10 @@ bool MessageReader::PopArrayOfObjectPaths(
 bool MessageReader::PopArrayOfBytesAsProto(
     google::protobuf::MessageLite* protobuf) {
   DCHECK(protobuf != NULL);
-  char* serialized_buf = NULL;
+  const char* serialized_buf = NULL;
   size_t buf_size = 0;
-  if (!PopArrayOfBytes(reinterpret_cast<uint8**>(&serialized_buf), &buf_size)) {
+  if (!PopArrayOfBytes(
+          reinterpret_cast<const uint8**>(&serialized_buf), &buf_size)) {
     LOG(ERROR) << "Error reading array of bytes";
     return false;
   }
diff --git a/dbus/message.h b/dbus/message.h
index 54ca036..db3456a 100644
--- a/dbus/message.h
+++ b/dbus/message.h
@@ -408,12 +408,14 @@ class CHROME_DBUS_EXPORT MessageReader {
   // Arrays of bytes are often used for exchanging binary blobs hence it's
   // worth having a specialized function.
   //
-  // |bytes| must be copied if the contents will be referenced after the
-  // MessageReader is destroyed.
-  bool PopArrayOfBytes(uint8** bytes, size_t* length);
-
-  // Gets the array of strings at the current iterator position.
-  // Returns true and advances the iterator on success.
+  // Ownership of the memory pointed to by |bytes| remains with the
+  // MessageReader; |bytes| must be copied if the contents will be referenced
+  // after the MessageReader is destroyed.
+  bool PopArrayOfBytes(const uint8** bytes, size_t* length);
+
+  // Gets the array of strings at the current iterator position. |strings| is
+  // cleared before being modified. Returns true and advances the iterator on
+  // success.
   //
   // Arrays of strings are often used to communicate with D-Bus
   // services like KWallet, hence it's worth having a specialized
@@ -421,7 +423,8 @@ class CHROME_DBUS_EXPORT MessageReader {
   bool PopArrayOfStrings(std::vector<std::string>* strings);
 
   // Gets the array of object paths at the current iterator position.
-  // Returns true and advances the iterator on success.
+  // |object_paths| is cleared before being modified. Returns true and advances
+  // the iterator on success.
   //
   // Arrays of object paths are often used to communicate with D-Bus
   // services like NetworkManager, hence it's worth having a specialized
diff --git a/dbus/message_unittest.cc b/dbus/message_unittest.cc
index 16348df..0c944d9 100644
--- a/dbus/message_unittest.cc
+++ b/dbus/message_unittest.cc
@@ -208,7 +208,7 @@ TEST(MessageTest, ArrayOfBytes) {
   writer.AppendArrayOfBytes(bytes.data(), bytes.size());
 
   MessageReader reader(message.get());
-  uint8* output_bytes = NULL;
+  const uint8* output_bytes = NULL;
   size_t length = 0;
   ASSERT_TRUE(reader.PopArrayOfBytes(&output_bytes, &length));
   ASSERT_FALSE(reader.HasMoreData());
@@ -225,7 +225,7 @@ TEST(MessageTest, ArrayOfBytes_Empty) {
   writer.AppendArrayOfBytes(bytes.data(), bytes.size());
 
   MessageReader reader(message.get());
-  uint8* output_bytes = NULL;
+  const uint8* output_bytes = NULL;
   size_t length = 0;
   ASSERT_TRUE(reader.PopArrayOfBytes(&output_bytes, &length));
   ASSERT_FALSE(reader.HasMoreData());