Return unique_ptr from CFX_BinaryBuf::DetachBuffer()

In turn, make CPDF_Stream() take an unique_ptr.

Review-Url: https://codereview.chromium.org/2584683002

8 files changed, 45 insertions, 40 deletions

diff --git a/core/fpdfapi/page/cpdf_image.cpp b/core/fpdfapi/page/cpdf_image.cpp
index 97969551c6..a0109f22a8 100644
--- a/core/fpdfapi/page/cpdf_image.cpp
+++ b/core/fpdfapi/page/cpdf_image.cpp
@@ -219,8 +219,9 @@ void CPDF_Image::SetImage(const CFX_DIBitmap* pBitmap) {
       pCS->AddNew<CPDF_Name>("Indexed");
       pCS->AddNew<CPDF_Name>("DeviceRGB");
       pCS->AddNew<CPDF_Number>(iPalette - 1);
-      uint8_t* pColorTable = FX_Alloc2D(uint8_t, iPalette, 3);
-      uint8_t* ptr = pColorTable;
+      std::unique_ptr<uint8_t, FxFreeDeleter> pColorTable(
+          FX_Alloc2D(uint8_t, iPalette, 3));
+      uint8_t* ptr = pColorTable.get();
       for (int32_t i = 0; i < iPalette; i++) {
         uint32_t argb = pBitmap->GetPaletteArgb(i);
         ptr[0] = (uint8_t)(argb >> 16);
@@ -231,7 +232,7 @@ void CPDF_Image::SetImage(const CFX_DIBitmap* pBitmap) {
       auto pNewDict =
           pdfium::MakeUnique<CPDF_Dictionary>(m_pDocument->GetByteStringPool());
       CPDF_Stream* pCTS = m_pDocument->NewIndirect<CPDF_Stream>(
-          pColorTable, iPalette * 3, std::move(pNewDict));
+          std::move(pColorTable), iPalette * 3, std::move(pNewDict));
       pCS->AddNew<CPDF_Reference>(m_pDocument, pCTS->GetObjNum());
       pDict->SetNewFor<CPDF_Reference>("ColorSpace", m_pDocument,
                                        pCS->GetObjNum());
@@ -254,7 +255,7 @@ void CPDF_Image::SetImage(const CFX_DIBitmap* pBitmap) {
   if (pMaskBitmap) {
     int32_t maskWidth = pMaskBitmap->GetWidth();
     int32_t maskHeight = pMaskBitmap->GetHeight();
-    uint8_t* mask_buf = nullptr;
+    std::unique_ptr<uint8_t, FxFreeDeleter> mask_buf;
     FX_STRSIZE mask_size = 0;
     auto pMaskDict =
         pdfium::MakeUnique<CPDF_Dictionary>(m_pDocument->GetByteStringPool());
@@ -265,16 +266,16 @@ void CPDF_Image::SetImage(const CFX_DIBitmap* pBitmap) {
     pMaskDict->SetNewFor<CPDF_Name>("ColorSpace", "DeviceGray");
     pMaskDict->SetNewFor<CPDF_Number>("BitsPerComponent", 8);
     if (pMaskBitmap->GetFormat() != FXDIB_1bppMask) {
-      mask_buf = FX_Alloc2D(uint8_t, maskHeight, maskWidth);
+      mask_buf.reset(FX_Alloc2D(uint8_t, maskHeight, maskWidth));
       mask_size = maskHeight * maskWidth;  // Safe since checked alloc returned.
       for (int32_t a = 0; a < maskHeight; a++) {
-        FXSYS_memcpy(mask_buf + a * maskWidth, pMaskBitmap->GetScanline(a),
-                     maskWidth);
+        FXSYS_memcpy(mask_buf.get() + a * maskWidth,
+                     pMaskBitmap->GetScanline(a), maskWidth);
       }
     }
     pMaskDict->SetNewFor<CPDF_Number>("Length", mask_size);
     CPDF_Stream* pNewStream = m_pDocument->NewIndirect<CPDF_Stream>(
-        mask_buf, mask_size, std::move(pMaskDict));
+        std::move(mask_buf), mask_size, std::move(pMaskDict));
     pDict->SetNewFor<CPDF_Reference>("SMask", m_pDocument,
                                      pNewStream->GetObjNum());
   }
diff --git a/core/fpdfapi/page/cpdf_streamparser.cpp b/core/fpdfapi/page/cpdf_streamparser.cpp
index fd5267b111..cf87ce9954 100644
--- a/core/fpdfapi/page/cpdf_streamparser.cpp
+++ b/core/fpdfapi/page/cpdf_streamparser.cpp
@@ -178,21 +178,22 @@ std::unique_ptr<CPDF_Stream> CPDF_StreamParser::ReadInlineStream(
     return nullptr;
 
   OrigSize *= height;
-  uint8_t* pData = nullptr;
+  std::unique_ptr<uint8_t, FxFreeDeleter> pData;
   uint32_t dwStreamSize;
   if (Decoder.IsEmpty()) {
     if (OrigSize > m_Size - m_Pos)
       OrigSize = m_Size - m_Pos;
-    pData = FX_Alloc(uint8_t, OrigSize);
-    FXSYS_memcpy(pData, m_pBuf + m_Pos, OrigSize);
+    pData.reset(FX_Alloc(uint8_t, OrigSize));
+    FXSYS_memcpy(pData.get(), m_pBuf + m_Pos, OrigSize);
     dwStreamSize = OrigSize;
     m_Pos += OrigSize;
   } else {
+    uint8_t* pIgnore;
     uint32_t dwDestSize = OrigSize;
     dwStreamSize =
         PDF_DecodeInlineStream(m_pBuf + m_Pos, m_Size - m_Pos, width, height,
-                               Decoder, pParam, pData, dwDestSize);
-    FX_Free(pData);
+                               Decoder, pParam, pIgnore, dwDestSize);
+    FX_Free(pIgnore);
     if (static_cast<int>(dwStreamSize) < 0)
       return nullptr;
 
@@ -216,12 +217,13 @@ std::unique_ptr<CPDF_Stream> CPDF_StreamParser::ReadInlineStream(
       dwStreamSize += m_Pos - dwPrevPos;
     }
     m_Pos = dwSavePos;
-    pData = FX_Alloc(uint8_t, dwStreamSize);
-    FXSYS_memcpy(pData, m_pBuf + m_Pos, dwStreamSize);
+    pData.reset(FX_Alloc(uint8_t, dwStreamSize));
+    FXSYS_memcpy(pData.get(), m_pBuf + m_Pos, dwStreamSize);
     m_Pos += dwStreamSize;
   }
   pDict->SetNewFor<CPDF_Number>("Length", (int)dwStreamSize);
-  return pdfium::MakeUnique<CPDF_Stream>(pData, dwStreamSize, std::move(pDict));
+  return pdfium::MakeUnique<CPDF_Stream>(std::move(pData), dwStreamSize,
+                                         std::move(pDict));
 }
 
 CPDF_StreamParser::SyntaxType CPDF_StreamParser::ParseNextElement() {
diff --git a/core/fpdfapi/parser/cpdf_object_unittest.cpp b/core/fpdfapi/parser/cpdf_object_unittest.cpp
index 303ad6fa7a..4977c9931f 100644
--- a/core/fpdfapi/parser/cpdf_object_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_object_unittest.cpp
@@ -69,14 +69,14 @@ class PDFObjectsTest : public testing::Test {
     // Stream object.
     const char content[] = "abcdefghijklmnopqrstuvwxyz";
     size_t buf_len = FX_ArraySize(content);
-    uint8_t* buf = reinterpret_cast<uint8_t*>(malloc(buf_len));
-    memcpy(buf, content, buf_len);
+    std::unique_ptr<uint8_t, FxFreeDeleter> buf(FX_Alloc(uint8_t, buf_len));
+    memcpy(buf.get(), content, buf_len);
     auto pNewDict = pdfium::MakeUnique<CPDF_Dictionary>();
     m_StreamDictObj = pNewDict.get();
     m_StreamDictObj->SetNewFor<CPDF_String>("key1", L" test dict");
     m_StreamDictObj->SetNewFor<CPDF_Number>("key2", -1);
     CPDF_Stream* stream_obj =
-        new CPDF_Stream(buf, buf_len, std::move(pNewDict));
+        new CPDF_Stream(std::move(buf), buf_len, std::move(pNewDict));
     // Null Object.
     CPDF_Null* null_obj = new CPDF_Null;
     // All direct objects.
@@ -588,9 +588,10 @@ TEST(PDFArrayTest, GetTypeAt) {
       }
       uint8_t content[] = "content: this is a stream";
       size_t data_size = FX_ArraySize(content);
-      uint8_t* data = reinterpret_cast<uint8_t*>(malloc(data_size));
-      memcpy(data, content, data_size);
-      stream_vals[i] = arr->AddNew<CPDF_Stream>(data, data_size,
+      std::unique_ptr<uint8_t, FxFreeDeleter> data(
+          FX_Alloc(uint8_t, data_size));
+      memcpy(data.get(), content, data_size);
+      stream_vals[i] = arr->AddNew<CPDF_Stream>(std::move(data), data_size,
                                                 pdfium::WrapUnique(vals[i]));
     }
     for (size_t i = 0; i < 3; ++i) {
@@ -634,10 +635,10 @@ TEST(PDFArrayTest, GetTypeAt) {
     // The data buffer will be owned by stream object, so it needs to be
     // dynamically allocated.
     size_t buf_size = sizeof(data);
-    uint8_t* buf = reinterpret_cast<uint8_t*>(malloc(buf_size));
-    memcpy(buf, data, buf_size);
+    std::unique_ptr<uint8_t, FxFreeDeleter> buf(FX_Alloc(uint8_t, buf_size));
+    memcpy(buf.get(), data, buf_size);
     CPDF_Stream* stream_val = arr->InsertNewAt<CPDF_Stream>(
-        13, buf, buf_size, pdfium::WrapUnique(stream_dict));
+        13, std::move(buf), buf_size, pdfium::WrapUnique(stream_dict));
     const char* const expected_str[] = {
         "true",          "false", "0",    "-1234", "2345", "0.05", "",
         "It is a test!", "NAME",  "test", "",      "",     "",     ""};
diff --git a/core/fpdfapi/parser/cpdf_stream.cpp b/core/fpdfapi/parser/cpdf_stream.cpp
index 3159791e82..e186bc19d1 100644
--- a/core/fpdfapi/parser/cpdf_stream.cpp
+++ b/core/fpdfapi/parser/cpdf_stream.cpp
@@ -18,10 +18,10 @@
 
 CPDF_Stream::CPDF_Stream() {}
 
-CPDF_Stream::CPDF_Stream(uint8_t* pData,
+CPDF_Stream::CPDF_Stream(std::unique_ptr<uint8_t, FxFreeDeleter> pData,
                          uint32_t size,
                          std::unique_ptr<CPDF_Dictionary> pDict)
-    : m_dwSize(size), m_pDict(std::move(pDict)), m_pDataBuf(pData) {}
+    : m_dwSize(size), m_pDict(std::move(pDict)), m_pDataBuf(std::move(pData)) {}
 
 CPDF_Stream::~CPDF_Stream() {
   m_ObjNum = kInvalidObjNum;
@@ -93,7 +93,7 @@ std::unique_ptr<CPDF_Object> CPDF_Stream::CloneNonCyclic(
     pNewDict = ToDictionary(
         static_cast<CPDF_Object*>(pDict)->CloneNonCyclic(bDirect, pVisited));
   }
-  return pdfium::MakeUnique<CPDF_Stream>(acc.DetachData().release(), streamSize,
+  return pdfium::MakeUnique<CPDF_Stream>(acc.DetachData(), streamSize,
                                          std::move(pNewDict));
 }
 
diff --git a/core/fpdfapi/parser/cpdf_stream.h b/core/fpdfapi/parser/cpdf_stream.h
index cd4113b22b..902cd75365 100644
--- a/core/fpdfapi/parser/cpdf_stream.h
+++ b/core/fpdfapi/parser/cpdf_stream.h
@@ -19,7 +19,7 @@ class CPDF_Stream : public CPDF_Object {
   CPDF_Stream();
 
   // Takes ownership of |pData|.
-  CPDF_Stream(uint8_t* pData,
+  CPDF_Stream(std::unique_ptr<uint8_t, FxFreeDeleter> pData,
               uint32_t size,
               std::unique_ptr<CPDF_Dictionary> pDict);
 
diff --git a/core/fpdfapi/parser/cpdf_syntax_parser.cpp b/core/fpdfapi/parser/cpdf_syntax_parser.cpp
index 2a20e43591..48d77c2cbd 100644
--- a/core/fpdfapi/parser/cpdf_syntax_parser.cpp
+++ b/core/fpdfapi/parser/cpdf_syntax_parser.cpp
@@ -723,25 +723,24 @@ std::unique_ptr<CPDF_Stream> CPDF_SyntaxParser::ReadStream(
   if (len < 0)
     return nullptr;
 
-  uint8_t* pData = nullptr;
+  std::unique_ptr<uint8_t, FxFreeDeleter> pData;
   if (len > 0) {
-    pData = FX_Alloc(uint8_t, len);
-    ReadBlock(pData, len);
+    pData.reset(FX_Alloc(uint8_t, len));
+    ReadBlock(pData.get(), len);
     if (pCryptoHandler) {
       CFX_BinaryBuf dest_buf;
       dest_buf.EstimateSize(pCryptoHandler->DecryptGetSize(len));
 
       void* context = pCryptoHandler->DecryptStart(objnum, gennum);
-      pCryptoHandler->DecryptStream(context, pData, len, dest_buf);
+      pCryptoHandler->DecryptStream(context, pData.get(), len, dest_buf);
       pCryptoHandler->DecryptFinish(context, dest_buf);
-      FX_Free(pData);
-      pData = dest_buf.GetBuffer();
       len = dest_buf.GetSize();
-      dest_buf.DetachBuffer();
+      pData = dest_buf.DetachBuffer();
     }
   }
 
-  auto pStream = pdfium::MakeUnique<CPDF_Stream>(pData, len, std::move(pDict));
+  auto pStream =
+      pdfium::MakeUnique<CPDF_Stream>(std::move(pData), len, std::move(pDict));
   streamStartPos = m_Pos;
   FXSYS_memset(m_WordBuffer, 0, kEndObjStr.GetLength() + 1);
   GetNextWordInternal(nullptr);
diff --git a/core/fxcrt/fx_basic.h b/core/fxcrt/fx_basic.h
index e974e3061c..7096e6f8df 100644
--- a/core/fxcrt/fx_basic.h
+++ b/core/fxcrt/fx_basic.h
@@ -41,7 +41,7 @@ class CFX_BinaryBuf {
   void Delete(int start_index, int count);
 
   // Releases ownership of |m_pBuffer| and returns it.
-  uint8_t* DetachBuffer();
+  std::unique_ptr<uint8_t, FxFreeDeleter> DetachBuffer();
 
  protected:
   void ExpandBuf(FX_STRSIZE size);
diff --git a/core/fxcrt/fx_basic_buffer.cpp b/core/fxcrt/fx_basic_buffer.cpp
index fcd156b079..14a85c2ee5 100644
--- a/core/fxcrt/fx_basic_buffer.cpp
+++ b/core/fxcrt/fx_basic_buffer.cpp
@@ -6,6 +6,8 @@
 
 #include <algorithm>
 #include <limits>
+#include <memory>
+#include <utility>
 
 #include "core/fxcrt/fx_basic.h"
 #include "core/fxcrt/fx_safe_types.h"
@@ -36,10 +38,10 @@ void CFX_BinaryBuf::Clear() {
   m_DataSize = 0;
 }
 
-uint8_t* CFX_BinaryBuf::DetachBuffer() {
+std::unique_ptr<uint8_t, FxFreeDeleter> CFX_BinaryBuf::DetachBuffer() {
   m_DataSize = 0;
   m_AllocSize = 0;
-  return m_pBuffer.release();
+  return std::move(m_pBuffer);
 }
 
 void CFX_BinaryBuf::EstimateSize(FX_STRSIZE size, FX_STRSIZE step) {