From aa326bd6b169dc1b5b9b83048c71799799ab34c6 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Wed, 6 Jan 2016 10:06:12 -0800 Subject: Return unique_ptrs from test_support functions R=thestig@chromium.org Review URL: https://codereview.chromium.org/1563513002 . --- fpdfsdk/src/fpdfdoc_embeddertest.cpp | 14 ++++++-------- fpdfsdk/src/fpdftext_embeddertest.cpp | 33 ++++++++++++++++----------------- samples/pdfium_test.cc | 9 ++++----- testing/embedder_test.cpp | 3 +-- testing/embedder_test.h | 3 ++- testing/test_support.cpp | 18 +++++++++++------- testing/test_support.h | 20 ++++++++++++++++---- 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 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(title)); - free(const_cast(bad_title)); + std::unique_ptr 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 nope = + GetFPDFWideString(L"nope"); + std::unique_ptr world = + GetFPDFWideString(L"world"); + std::unique_ptr world_caps = + GetFPDFWideString(L"WORLD"); + std::unique_ptr 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(nope)); - free(const_cast(world)); - free(const_cast(world_caps)); - free(const_cast(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 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(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 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 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 buffer( + static_cast(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 GetFPDFWideString( + const std::wstring& wstr) { size_t length = sizeof(uint16_t) * (wstr.length() + 1); - unsigned char* ptr = static_cast(malloc(length)); + std::unique_ptr result( + static_cast(malloc(length))); + char* ptr = reinterpret_cast(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(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 +#include #include #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 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 GetFPDFWideString( + const std::wstring& wstr); #ifdef PDF_ENABLE_V8 #ifdef V8_USE_EXTERNAL_STARTUP_DATA -- cgit v1.2.3