debugd: Fix memory leaks in DBusUtils unit tests.
The DBusUtils unit tests previously did not free the base::Value objects
returned by debugd::DBusMessageToValue(), which causes ASAN to complain.
This CL fixes the issue in the unit tests and restructures the code.
BUG=chromium:408106
TEST=`FEATURES=test P2_TEST_FILTER='debugd::*' platform2`
TEST=`USE='clang asan' FEATURES=test P2_TEST_FILTER='debugd::*' platform2`
Change-Id: I4e9e726114ffc50e9c3c916160889f176fd7ccc8
Reviewed-on: https://chromium-review.googlesource.com/214569
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Tested-by: Ben Chan <benchan@chromium.org>
Commit-Queue: Ben Chan <benchan@chromium.org>
diff --git a/debugd/src/dbus_utils_unittest.cc b/debugd/src/dbus_utils_unittest.cc
index 1a70b97..fa55d92 100644
--- a/debugd/src/dbus_utils_unittest.cc
+++ b/debugd/src/dbus_utils_unittest.cc
@@ -5,6 +5,7 @@
#include "debugd/src/dbus_utils.h"
#include <limits>
+#include <memory>
#include <vector>
#include <base/strings/string_number_conversions.h>
@@ -12,88 +13,73 @@
#include <dbus-c++/dbus.h>
#include <gtest/gtest.h>
-
using base::DictionaryValue;
using base::ListValue;
using base::Value;
-static void MessageToValues(const DBus::Message& m, std::vector<Value*>* result,
- bool* success) {
- Value* v = NULL;
- *success = false;
- ASSERT_TRUE(debugd::DBusMessageToValue(m, &v));
- ASSERT_TRUE(v != NULL);
- ASSERT_EQ(Value::TYPE_LIST, v->GetType());
- ListValue* lv = NULL;
- ASSERT_TRUE(v->GetAsList(&lv));
- std::vector<Value*> values;
- for (size_t i = 0; i < lv->GetSize(); i++) {
- Value* v0 = NULL;
- ASSERT_TRUE(lv->Get(i, &v0));
- values.push_back(v0);
+class DBusUtilsTest : public testing::Test {
+ protected:
+ void ConvertDBusMessageToValues() {
+ Value* value = NULL;
+ ASSERT_TRUE(debugd::DBusMessageToValue(message_, &value));
+ ASSERT_TRUE(value != NULL);
+ raw_value_.reset(value);
+ ASSERT_EQ(Value::TYPE_LIST, value->GetType());
+ ListValue* list_values = NULL;
+ ASSERT_TRUE(value->GetAsList(&list_values));
+ values_.clear();
+ for (size_t i = 0; i < list_values->GetSize(); i++) {
+ Value* element = NULL;
+ ASSERT_TRUE(list_values->Get(i, &element));
+ values_.push_back(element);
+ }
}
- *result = values;
- *success = true;
+
+ DBus::CallMessage message_;
+ std::unique_ptr<Value> raw_value_;
+ std::vector<Value*> values_;
+};
+
+TEST_F(DBusUtilsTest, DBusMessageToValueEmpty) {
+ ConvertDBusMessageToValues();
+ EXPECT_EQ(0, values_.size());
}
-static void MessageToOneValue(const DBus::Message& m, Value** result,
- bool* success) {
- std::vector<Value*> values;
- MessageToValues(m, &values, success);
- ASSERT_EQ(1, values.size());
- *result = values[0];
- *success = true;
-}
-
-TEST(Utilities, DBusMessageToValueEmpty) {
- DBus::CallMessage m;
- std::vector<Value*> values;
- bool ok = false;
- MessageToValues(m, &values, &ok);
- ASSERT_TRUE(ok);
- EXPECT_EQ(0, values.size());
-}
-
-TEST(Utilities, DBusMessageToValueOnePrimitive) {
- DBus::CallMessage m;
- DBus::MessageIter iter = m.writer();
+TEST_F(DBusUtilsTest, DBusMessageToValueOnePrimitive) {
+ DBus::MessageIter iter = message_.writer();
iter.append_bool(true);
- Value* v = NULL;
- bool ok = false;
- MessageToOneValue(m, &v, &ok);
- ASSERT_TRUE(ok);
+
+ ConvertDBusMessageToValues();
+ ASSERT_EQ(1, values_.size());
+
+ Value* v = values_[0];
EXPECT_EQ(Value::TYPE_BOOLEAN, v->GetType());
- bool b;
+ bool b = false;
EXPECT_TRUE(v->GetAsBoolean(&b));
EXPECT_TRUE(b);
}
-TEST(Utilities, DBusMessageToValueMultiplePrimitives) {
- DBus::CallMessage m;
- DBus::MessageIter iter = m.writer();
+TEST_F(DBusUtilsTest, DBusMessageToValueMultiplePrimitives) {
+ DBus::MessageIter iter = message_.writer();
int v0_old = -3;
int v1_old = 17;
iter.append_int32(v0_old);
iter.append_int32(v1_old);
- std::vector<Value*> values;
- bool ok = false;
- MessageToValues(m, &values, &ok);
- ASSERT_TRUE(ok);
- ASSERT_EQ(2, values.size());
+ ConvertDBusMessageToValues();
+ ASSERT_EQ(2, values_.size());
- int v0_new;
- EXPECT_TRUE(values[0]->GetAsInteger(&v0_new));
+ int v0_new = 0;
+ EXPECT_TRUE(values_[0]->GetAsInteger(&v0_new));
EXPECT_EQ(v0_old, v0_new);
- int v1_new;
- EXPECT_TRUE(values[1]->GetAsInteger(&v1_new));
+ int v1_new = 0;
+ EXPECT_TRUE(values_[1]->GetAsInteger(&v1_new));
EXPECT_EQ(v1_old, v1_new);
}
-TEST(Utilities, DBusMessageToValueArray) {
- DBus::CallMessage m;
- DBus::MessageIter iter = m.writer();
+TEST_F(DBusUtilsTest, DBusMessageToValueArray) {
+ DBus::MessageIter iter = message_.writer();
DBus::MessageIter subiter = iter.new_array(DBUS_TYPE_INT32_AS_STRING);
int v0_old = 7;
int v1_old = -2;
@@ -101,10 +87,10 @@
subiter.append_int32(v1_old);
iter.close_container(subiter);
- Value* v = NULL;
- bool ok = false;
- MessageToOneValue(m, &v, &ok);
- ASSERT_TRUE(ok);
+ ConvertDBusMessageToValues();
+ ASSERT_EQ(1, values_.size());
+
+ Value* v = values_[0];
ASSERT_EQ(Value::TYPE_LIST, v->GetType());
ListValue* lv = NULL;
@@ -127,7 +113,7 @@
EXPECT_EQ(v1_old, v1_new);
}
-TEST(Utilities, DBusMessageToValueDictionary) {
+TEST_F(DBusUtilsTest, DBusMessageToValueDictionary) {
char entry_type[5] = {
DBUS_DICT_ENTRY_BEGIN_CHAR,
DBUS_TYPE_STRING,
@@ -135,8 +121,7 @@
DBUS_DICT_ENTRY_END_CHAR,
'\0'
};
- DBus::CallMessage m;
- DBus::MessageIter iter = m.writer();
+ DBus::MessageIter iter = message_.writer();
DBus::MessageIter subiter = iter.new_array(entry_type);
DBus::MessageIter subsubiter = subiter.new_dict_entry();
int foo_old = 17;
@@ -150,14 +135,13 @@
subiter.close_container(subsubiter);
iter.close_container(subiter);
- Value* v = NULL;
- bool ok = false;
- MessageToOneValue(m, &v, &ok);
- ASSERT_TRUE(ok);
- ASSERT_EQ(Value::TYPE_DICTIONARY, v->GetType());
+ ConvertDBusMessageToValues();
+ ASSERT_EQ(1, values_.size());
- // TODO(ellyjones): There should be a GetAsDictionary() method on Value.
- DictionaryValue* dv = static_cast<DictionaryValue*>(v);
+ Value* v = values_[0];
+ EXPECT_EQ(Value::TYPE_DICTIONARY, v->GetType());
+ DictionaryValue* dv = NULL;
+ ASSERT_TRUE(v->GetAsDictionary(&dv));
int foo_new;
ASSERT_TRUE(dv->GetInteger("foo", &foo_new));
EXPECT_EQ(foo_old, foo_new);
@@ -166,27 +150,25 @@
EXPECT_EQ(bar_old, bar_new);
}
-TEST(Utilities, DBusMessageToValueIntTypes) {
- DBus::CallMessage m;
- DBus::MessageIter iter = m.writer();
+TEST_F(DBusUtilsTest, DBusMessageToValueIntTypes) {
+ DBus::MessageIter iter = message_.writer();
uint32_t uint32_max_old = std::numeric_limits<uint32_t>::max();
iter.append_uint32(uint32_max_old);
int64_t int64_min_old = std::numeric_limits<int64_t>::min();
iter.append_int64(int64_min_old);
uint64_t uint64_max_old = std::numeric_limits<uint64_t>::max();
iter.append_uint64(uint64_max_old);
- std::vector<Value*> values;
- bool ok = false;
- MessageToValues(m, &values, &ok);
- ASSERT_TRUE(ok);
- ASSERT_EQ(3, values.size());
+
+ ConvertDBusMessageToValues();
+ ASSERT_EQ(3, values_.size());
+
std::string uint32_max_new;
std::string int64_min_new;
std::string uint64_max_new;
- ASSERT_TRUE(values[0]->GetAsString(&uint32_max_new));
+ ASSERT_TRUE(values_[0]->GetAsString(&uint32_max_new));
EXPECT_EQ(base::UintToString(uint32_max_old), uint32_max_new);
- ASSERT_TRUE(values[1]->GetAsString(&int64_min_new));
+ ASSERT_TRUE(values_[1]->GetAsString(&int64_min_new));
EXPECT_EQ(base::Int64ToString(int64_min_old), int64_min_new);
- ASSERT_TRUE(values[2]->GetAsString(&uint64_max_new));
+ ASSERT_TRUE(values_[2]->GetAsString(&uint64_max_new));
EXPECT_EQ(base::Uint64ToString(uint64_max_old), uint64_max_new);
}
