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
CrOS-Libchrome-Original-Commit: 38715910ea552ad5198b6dba8bca3a57f46c1e9a
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 @@
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::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::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::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 @@
// 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);
+ // 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.
- // Returns true and advances the iterator on success.
+ // 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 @@
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 @@
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 @@
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());