commit 30c7f48b04e6ed2fa358225eadf6a33ff7374716
author Christopher Ferris <cferris@google.com> Mon Jun 19 13:42:04 2017 -0700
committer Christopher Ferris <cferris@google.com> Mon Jun 19 16:20:33 2017 -0700
tree 7a0b4848ae3b5e87c453784352dd5d9a0f70eccd
parent 38e39936a43a3508e0da687437411e6991a299b8

Fix some memory leaks, potential errors.

Fix a couple of memory leaks in tests.

Fix a potential case that length could be zero.

Bug: 23762183

Test: Ran unit tests, then ran valgrind on unit tests and verified no leaks.
Change-Id: I5f7602aa0204db0488a8e305250658cdb6e977d0

libunwindstack/Maps.cpp

diff --git a/libunwindstack/Maps.cpp b/libunwindstack/Maps.cpp
index b369c43..b869918 100644
--- a/libunwindstack/Maps.cpp
+++ b/libunwindstack/Maps.cpp
@@ -71,15 +71,17 @@
     map_info->flags |= PROT_EXEC;
   }
 
-  map_info->name = &line[name_pos];
-  size_t length = map_info->name.length() - 1;
-  if (map_info->name[length] == '\n') {
-    map_info->name.erase(length);
-  }
-  // Mark a device map in /dev/and not in /dev/ashmem/ specially.
-  if (!map_info->name.empty() && map_info->name.substr(0, 5) == "/dev/" &&
-      map_info->name.substr(5, 7) != "ashmem/") {
-    map_info->flags |= MAPS_FLAGS_DEVICE_MAP;
+  if (line[name_pos] != '\0') {
+    map_info->name = &line[name_pos];
+    size_t length = map_info->name.length() - 1;
+    if (map_info->name[length] == '\n') {
+      map_info->name.erase(length);
+    }
+
+    // Mark a device map in /dev/and not in /dev/ashmem/ specially.
+    if (map_info->name.substr(0, 5) == "/dev/" && map_info->name.substr(5, 7) != "ashmem/") {
+      map_info->flags |= MAPS_FLAGS_DEVICE_MAP;
+    }
   }
 
   return true;

libunwindstack/tests/MapInfoTest.cpp

diff --git a/libunwindstack/tests/MapInfoTest.cpp b/libunwindstack/tests/MapInfoTest.cpp
index c846ad7..6e47dc0 100644
--- a/libunwindstack/tests/MapInfoTest.cpp
+++ b/libunwindstack/tests/MapInfoTest.cpp
@@ -75,7 +75,7 @@
   // Make sure this test is valid.
   info.end = 0x101;
   memory.reset(info.CreateMemory(getpid()));
-  ASSERT_FALSE(info.CreateMemory(getpid()) == nullptr);
+  ASSERT_TRUE(memory.get() != nullptr);
 }
 
 // Verify that if the offset is non-zero but there is no elf at the offset,
@@ -212,8 +212,8 @@
   MapInfo info{.start = start, .end = start + 1024, .offset = 0, .name = ""};
 
   // The map contains garbage, but this should still produce an elf object.
-  Elf* elf = info.GetElf(getpid(), false);
-  ASSERT_TRUE(elf != nullptr);
+  std::unique_ptr<Elf> elf(info.GetElf(getpid(), false));
+  ASSERT_TRUE(elf.get() != nullptr);
   ASSERT_FALSE(elf->valid());
 
   ASSERT_EQ(0, munmap(map, 1024));

libunwindstack/tests/MemoryRangeTest.cpp

diff --git a/libunwindstack/tests/MemoryRangeTest.cpp b/libunwindstack/tests/MemoryRangeTest.cpp
index ee5ba01..469afcc 100644
--- a/libunwindstack/tests/MemoryRangeTest.cpp
+++ b/libunwindstack/tests/MemoryRangeTest.cpp
@@ -26,21 +26,13 @@
 
 #include "MemoryFake.h"
 
-class MemoryRangeTest : public ::testing::Test {
- protected:
-  void SetUp() override {
-    memory_ = new MemoryFake;
-  }
-
-  MemoryFake* memory_;
-};
-
-TEST_F(MemoryRangeTest, read) {
+TEST(MemoryRangeTest, read) {
   std::vector<uint8_t> src(1024);
   memset(src.data(), 0x4c, 1024);
-  memory_->SetMemory(9001, src);
+  MemoryFake* memory = new MemoryFake;
+  memory->SetMemory(9001, src);
 
-  MemoryRange range(memory_, 9001, 9001 + src.size());
+  MemoryRange range(memory, 9001, 9001 + src.size());
 
   std::vector<uint8_t> dst(1024);
   ASSERT_TRUE(range.Read(0, dst.data(), src.size()));
@@ -49,12 +41,13 @@
   }
 }
 
-TEST_F(MemoryRangeTest, read_near_limit) {
+TEST(MemoryRangeTest, read_near_limit) {
   std::vector<uint8_t> src(4096);
   memset(src.data(), 0x4c, 4096);
-  memory_->SetMemory(1000, src);
+  MemoryFake* memory = new MemoryFake;
+  memory->SetMemory(1000, src);
 
-  MemoryRange range(memory_, 1000, 2024);
+  MemoryRange range(memory, 1000, 2024);
 
   std::vector<uint8_t> dst(1024);
   ASSERT_TRUE(range.Read(1020, dst.data(), 4));
@@ -71,7 +64,7 @@
   ASSERT_TRUE(range.Read(1020, dst.data(), 4));
 }
 
-TEST_F(MemoryRangeTest, read_overflow) {
+TEST(MemoryRangeTest, read_overflow) {
   std::vector<uint8_t> buffer(100);
 
   std::unique_ptr<MemoryRange> overflow(new MemoryRange(new MemoryFakeAlwaysReadZero, 100, 200));