Return unique_ptrs from test_support functions

R=thestig@chromium.org

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

7 files changed, 56 insertions, 44 deletions

diff --git a/fpdfsdk/src/fpdfdoc_embeddertest.cpp b/fpdfsdk/src/fpdfdoc_embeddertest.cpp
index 260f25f309..1c66a15bad 100644
--- a/fpdfsdk/src/fpdfdoc_embeddertest.cpp
+++ b/fpdfsdk/src/fpdfdoc_embeddertest.cpp
@@ -106,8 +106,9 @@ TEST_F(FPDFDocEmbeddertest, FindBookmarks) {
   EXPECT_TRUE(OpenDocument("bookmarks.pdf"));
 
   // Find the first one, based on its known title.
-  FPDF_WIDESTRING title = GetFPDFWideString(L"A Good Beginning");
-  FPDF_BOOKMARK child = FPDFBookmark_Find(document(), title);
+  std::unique_ptr<unsigned short, pdfium::FreeDeleter> title =
+      GetFPDFWideString(L"A Good Beginning");
+  FPDF_BOOKMARK child = FPDFBookmark_Find(document(), title.get());
   EXPECT_NE(nullptr, child);
 
   // Check that the string matches.
@@ -120,10 +121,7 @@ TEST_F(FPDFDocEmbeddertest, FindBookmarks) {
   EXPECT_EQ(child, FPDFBookmark_GetFirstChild(document(), nullptr));
 
   // Try to find one using a non-existent title.
-  FPDF_WIDESTRING bad_title = GetFPDFWideString(L"A BAD Beginning");
-  EXPECT_EQ(nullptr, FPDFBookmark_Find(document(), bad_title));
-
-  // Alas, the typedef includes the "const".
-  free(const_cast<unsigned short*>(title));
-  free(const_cast<unsigned short*>(bad_title));
+  std::unique_ptr<unsigned short, pdfium::FreeDeleter> bad_title =
+      GetFPDFWideString(L"A BAD Beginning");
+  EXPECT_EQ(nullptr, FPDFBookmark_Find(document(), bad_title.get()));
 }
diff --git a/fpdfsdk/src/fpdftext_embeddertest.cpp b/fpdfsdk/src/fpdftext_embeddertest.cpp
index 3772686c88..4653db32ad 100644
--- a/fpdfsdk/src/fpdftext_embeddertest.cpp
+++ b/fpdfsdk/src/fpdftext_embeddertest.cpp
@@ -144,13 +144,17 @@ TEST_F(FPDFTextEmbeddertest, TextSearch) {
   FPDF_TEXTPAGE textpage = FPDFText_LoadPage(page);
   EXPECT_NE(nullptr, textpage);
 
-  FPDF_WIDESTRING nope = GetFPDFWideString(L"nope");
-  FPDF_WIDESTRING world = GetFPDFWideString(L"world");
-  FPDF_WIDESTRING world_caps = GetFPDFWideString(L"WORLD");
-  FPDF_WIDESTRING world_substr = GetFPDFWideString(L"orld");
+  std::unique_ptr<unsigned short, pdfium::FreeDeleter> nope =
+      GetFPDFWideString(L"nope");
+  std::unique_ptr<unsigned short, pdfium::FreeDeleter> world =
+      GetFPDFWideString(L"world");
+  std::unique_ptr<unsigned short, pdfium::FreeDeleter> world_caps =
+      GetFPDFWideString(L"WORLD");
+  std::unique_ptr<unsigned short, pdfium::FreeDeleter> world_substr =
+      GetFPDFWideString(L"orld");
 
   // No occurences of "nope" in test page.
-  FPDF_SCHHANDLE search = FPDFText_FindStart(textpage, nope, 0, 0);
+  FPDF_SCHHANDLE search = FPDFText_FindStart(textpage, nope.get(), 0, 0);
   EXPECT_NE(nullptr, search);
   EXPECT_EQ(0, FPDFText_GetSchResultIndex(search));
   EXPECT_EQ(0, FPDFText_GetSchCount(search));
@@ -167,7 +171,7 @@ TEST_F(FPDFTextEmbeddertest, TextSearch) {
   FPDFText_FindClose(search);
 
   // Two occurences of "world" in test page.
-  search = FPDFText_FindStart(textpage, world, 0, 2);
+  search = FPDFText_FindStart(textpage, world.get(), 0, 2);
   EXPECT_NE(nullptr, search);
 
   // Remains not found until advanced.
@@ -201,7 +205,7 @@ TEST_F(FPDFTextEmbeddertest, TextSearch) {
   FPDFText_FindClose(search);
 
   // Exact search unaffected by case sensitiity and whole word flags.
-  search = FPDFText_FindStart(textpage, world,
+  search = FPDFText_FindStart(textpage, world.get(),
                               FPDF_MATCHCASE | FPDF_MATCHWHOLEWORD, 0);
   EXPECT_NE(nullptr, search);
   EXPECT_TRUE(FPDFText_FindNext(search));
@@ -210,7 +214,7 @@ TEST_F(FPDFTextEmbeddertest, TextSearch) {
   FPDFText_FindClose(search);
 
   // Default is case-insensitive, so matching agaist caps works.
-  search = FPDFText_FindStart(textpage, world_caps, 0, 0);
+  search = FPDFText_FindStart(textpage, world_caps.get(), 0, 0);
   EXPECT_NE(nullptr, search);
   EXPECT_TRUE(FPDFText_FindNext(search));
   EXPECT_EQ(7, FPDFText_GetSchResultIndex(search));
@@ -218,33 +222,28 @@ TEST_F(FPDFTextEmbeddertest, TextSearch) {
   FPDFText_FindClose(search);
 
   // But can be made case sensitive, in which case this fails.
-  search = FPDFText_FindStart(textpage, world_caps, FPDF_MATCHCASE, 0);
+  search = FPDFText_FindStart(textpage, world_caps.get(), FPDF_MATCHCASE, 0);
   EXPECT_FALSE(FPDFText_FindNext(search));
   EXPECT_EQ(0, FPDFText_GetSchResultIndex(search));
   EXPECT_EQ(0, FPDFText_GetSchCount(search));
   FPDFText_FindClose(search);
 
   // Default is match anywhere within word, so matching substirng works.
-  search = FPDFText_FindStart(textpage, world_substr, 0, 0);
+  search = FPDFText_FindStart(textpage, world_substr.get(), 0, 0);
   EXPECT_TRUE(FPDFText_FindNext(search));
   EXPECT_EQ(8, FPDFText_GetSchResultIndex(search));
   EXPECT_EQ(4, FPDFText_GetSchCount(search));
   FPDFText_FindClose(search);
 
   // But can be made to mach word boundaries, in which case this fails.
-  search = FPDFText_FindStart(textpage, world_substr, FPDF_MATCHWHOLEWORD, 0);
+  search =
+      FPDFText_FindStart(textpage, world_substr.get(), FPDF_MATCHWHOLEWORD, 0);
   EXPECT_FALSE(FPDFText_FindNext(search));
   // TODO(tsepez): investigate strange index/count values in this state.
   FPDFText_FindClose(search);
 
   FPDFText_ClosePage(textpage);
   UnloadPage(page);
-
-  // Alas, the typedef includes the "const".
-  free(const_cast<unsigned short*>(nope));
-  free(const_cast<unsigned short*>(world));
-  free(const_cast<unsigned short*>(world_caps));
-  free(const_cast<unsigned short*>(world_substr));
 }
 
 // Test that the page has characters despite a bad stream length.
diff --git a/samples/pdfium_test.cc b/samples/pdfium_test.cc
index 9816bb8a31..c522a318e6 100644
--- a/samples/pdfium_test.cc
+++ b/samples/pdfium_test.cc
@@ -590,11 +590,10 @@ int main(int argc, const char* argv[]) {
     std::string filename = files.front();
     files.pop_front();
     size_t file_length = 0;
-    char* file_contents = GetFileContents(filename.c_str(), &file_length);
-    if (!file_contents)
-      continue;
-    RenderPdf(filename, file_contents, file_length, options);
-    free(file_contents);
+    std::unique_ptr<char, pdfium::FreeDeleter> file_contents =
+        GetFileContents(filename.c_str(), &file_length);
+    if (file_contents)
+      RenderPdf(filename, file_contents.get(), file_length, options);
   }
 
   FPDF_DestroyLibrary();
diff --git a/testing/embedder_test.cpp b/testing/embedder_test.cpp
index e99c2e351a..cb9068564a 100644
--- a/testing/embedder_test.cpp
+++ b/testing/embedder_test.cpp
@@ -90,7 +90,6 @@ void EmbedderTest::TearDown() {
 #endif  // PDF_ENABLE_V8
 
   delete loader_;
-  free(file_contents_);
 }
 
 bool EmbedderTest::CreateEmptyDocument() {
@@ -111,7 +110,7 @@ bool EmbedderTest::OpenDocument(const std::string& filename,
   if (!file_contents_)
     return false;
 
-  loader_ = new TestLoader(file_contents_, file_length_);
+  loader_ = new TestLoader(file_contents_.get(), file_length_);
   file_access_.m_FileLen = static_cast<unsigned long>(file_length_);
   file_access_.m_GetBlock = TestLoader::GetBlock;
   file_access_.m_Param = loader_;
diff --git a/testing/embedder_test.h b/testing/embedder_test.h
index c780cca74b..a21a55de2c 100644
--- a/testing/embedder_test.h
+++ b/testing/embedder_test.h
@@ -14,6 +14,7 @@
 #include "public/fpdf_formfill.h"
 #include "public/fpdfview.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "testing/test_support.h"
 
 #ifdef PDF_ENABLE_V8
 #include "v8/include/v8.h"
@@ -129,7 +130,7 @@ class EmbedderTest : public ::testing::Test,
   void* external_isolate_;
   TestLoader* loader_;
   size_t file_length_;
-  char* file_contents_;
+  std::unique_ptr<char, pdfium::FreeDeleter> file_contents_;
 
  private:
   static void UnsupportedHandlerTrampoline(UNSUPPORT_INFO*, int type);
diff --git a/testing/test_support.cpp b/testing/test_support.cpp
index c7ab10ee3c..ce143ae8e9 100644
--- a/testing/test_support.cpp
+++ b/testing/test_support.cpp
@@ -71,7 +71,8 @@ void InitializeV8Common(v8::Platform** platform) {
 
 }  // namespace
 
-char* GetFileContents(const char* filename, size_t* retlen) {
+std::unique_ptr<char, pdfium::FreeDeleter> GetFileContents(const char* filename,
+                                                           size_t* retlen) {
   FILE* file = fopen(filename, "rb");
   if (!file) {
     fprintf(stderr, "Failed to open: %s\n", filename);
@@ -83,15 +84,15 @@ char* GetFileContents(const char* filename, size_t* retlen) {
     return nullptr;
   }
   (void)fseek(file, 0, SEEK_SET);
-  char* buffer = (char*)malloc(file_length);
+  std::unique_ptr<char, pdfium::FreeDeleter> buffer(
+      static_cast<char*>(malloc(file_length)));
   if (!buffer) {
     return nullptr;
   }
-  size_t bytes_read = fread(buffer, 1, file_length, file);
+  size_t bytes_read = fread(buffer.get(), 1, file_length, file);
   (void)fclose(file);
   if (bytes_read != file_length) {
     fprintf(stderr, "Failed to read: %s\n", filename);
-    free(buffer);
     return nullptr;
   }
   *retlen = bytes_read;
@@ -114,9 +115,12 @@ std::wstring GetPlatformWString(FPDF_WIDESTRING wstr) {
   return platform_string;
 }
 
-FPDF_WIDESTRING GetFPDFWideString(const std::wstring& wstr) {
+std::unique_ptr<unsigned short, pdfium::FreeDeleter> GetFPDFWideString(
+    const std::wstring& wstr) {
   size_t length = sizeof(uint16_t) * (wstr.length() + 1);
-  unsigned char* ptr = static_cast<unsigned char*>(malloc(length));
+  std::unique_ptr<unsigned short, pdfium::FreeDeleter> result(
+      static_cast<unsigned short*>(malloc(length)));
+  char* ptr = reinterpret_cast<char*>(result.get());
   size_t i = 0;
   for (wchar_t w : wstr) {
     ptr[i++] = w & 0xff;
@@ -124,7 +128,7 @@ FPDF_WIDESTRING GetFPDFWideString(const std::wstring& wstr) {
   }
   ptr[i++] = 0;
   ptr[i] = 0;
-  return reinterpret_cast<FPDF_WIDESTRING>(ptr);
+  return result;
 }
 
 #ifdef PDF_ENABLE_V8
diff --git a/testing/test_support.h b/testing/test_support.h
index d48d5596f4..945704194d 100644
--- a/testing/test_support.h
+++ b/testing/test_support.h
@@ -6,6 +6,7 @@
 #define TESTING_EMBEDDER_TEST_SUPPORT_H_
 
 #include <stdlib.h>
+#include <memory>
 #include <string>
 
 #include "public/fpdfview.h"
@@ -14,16 +15,27 @@
 #include "v8/include/v8.h"
 #endif  // PDF_ENABLE_V8
 
-// Reads the entire contents of a file into a newly malloc'd buffer.
-char* GetFileContents(const char* filename, size_t* retlen);
+namespace pdfium {
+
+// Used with std::unique_ptr to free() objects that can't be deleted.
+struct FreeDeleter {
+  inline void operator()(void* ptr) const { free(ptr); }
+};
+
+}  // namespace pdfium
+
+// Reads the entire contents of a file into a newly alloc'd buffer.
+std::unique_ptr<char, pdfium::FreeDeleter> GetFileContents(const char* filename,
+                                                           size_t* retlen);
 
 // Converts a FPDF_WIDESTRING to a std::wstring.
 // Deals with differences between UTF16LE and wchar_t.
 std::wstring GetPlatformWString(const FPDF_WIDESTRING wstr);
 
-// Returns a newly mallocated FPDF_WIDESTRING (caller must free()).
+// Returns a newly allocated FPDF_WIDESTRING.
 // Deals with differences between UTF16LE and wchar_t.
-FPDF_WIDESTRING GetFPDFWideString(const std::wstring& wstr);
+std::unique_ptr<unsigned short, pdfium::FreeDeleter> GetFPDFWideString(
+    const std::wstring& wstr);
 
 #ifdef PDF_ENABLE_V8
 #ifdef V8_USE_EXTERNAL_STARTUP_DATA