Use CFX_MaybeOwned<> in fpdf_edit_create.cpp

core/fpdfapi/edit/fpdf_edit_create.cpp

 // Copyright 2014 PDFium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include "core/fpdfapi/edit/editint.h"
 
+#include <memory>
 #include <vector>
 
 #include "core/fpdfapi/edit/cpdf_creator.h"
 #include "core/fpdfapi/parser/cpdf_array.h"
 #include "core/fpdfapi/parser/cpdf_crypto_handler.h"
 #include "core/fpdfapi/parser/cpdf_dictionary.h"
 #include "core/fpdfapi/parser/cpdf_document.h"
 #include "core/fpdfapi/parser/cpdf_name.h"
 #include "core/fpdfapi/parser/cpdf_number.h"
 #include "core/fpdfapi/parser/cpdf_parser.h"
 #include "core/fpdfapi/parser/cpdf_reference.h"
 #include "core/fpdfapi/parser/cpdf_security_handler.h"
 #include "core/fpdfapi/parser/cpdf_stream.h"
 #include "core/fpdfapi/parser/cpdf_stream_acc.h"
 #include "core/fpdfapi/parser/cpdf_string.h"
 #include "core/fpdfapi/parser/fpdf_parser_decode.h"
+#include "core/fxcrt/cfx_maybe_owned.h"
 #include "core/fxcrt/fx_ext.h"
 #include "third_party/base/stl_util.h"
 
 #define PDF_OBJECTSTREAM_MAXLENGTH (256 * 1024)
 #define PDF_XREFSTREAM_MAXSIZE 10000
 
 // TODO(ochang): Make helper for appending "objnum 0 R ".
 
 namespace {
 
@@ skipping 354 matching lines @@
  public:
   CPDF_FlateEncoder(CPDF_Stream* pStream, bool bFlateEncode);
   CPDF_FlateEncoder(const uint8_t* pBuffer,
                     uint32_t size,
                     bool bFlateEncode,
                     bool bXRefStream);
   ~CPDF_FlateEncoder();
 
   void CloneDict();
 
-  uint8_t* m_pData;
   uint32_t m_dwSize;
-  CPDF_Dictionary* m_pDict;
-  bool m_bCloned;
-  bool m_bNewData;
+  CFX_MaybeOwned<uint8_t, FxFreeDeleter> m_pData;
+  CFX_MaybeOwned<CPDF_Dictionary> m_pDict;
   CPDF_StreamAcc m_Acc;
 };
 
 void CPDF_FlateEncoder::CloneDict() {
-  if (!m_bCloned) {
-    m_pDict = ToDictionary(m_pDict->Clone().release());
-    ASSERT(m_pDict);
-    m_bCloned = true;
-  }
+  if (m_pDict.IsOwned())
+    return;
+  m_pDict = ToDictionary(m_pDict->Clone());
+  ASSERT(m_pDict.IsOwned());
 }
 
 CPDF_FlateEncoder::CPDF_FlateEncoder(CPDF_Stream* pStream, bool bFlateEncode)
-    : m_pData(nullptr),
-      m_dwSize(0),
-      m_pDict(nullptr),
-      m_bCloned(false),
-      m_bNewData(false) {
+    : m_dwSize(0) {
   m_Acc.LoadAllData(pStream, true);
   bool bHasFilter = pStream && pStream->HasFilter();
   if (bHasFilter && !bFlateEncode) {
     CPDF_StreamAcc destAcc;
     destAcc.LoadAllData(pStream);
     m_dwSize = destAcc.GetSize();
-    m_pData = (uint8_t*)destAcc.DetachData();
-    m_pDict = ToDictionary(pStream->GetDict()->Clone().release());
+    m_pData = destAcc.DetachData();
+    m_pDict = ToDictionary(pStream->GetDict()->Clone());
     m_pDict->RemoveFor("Filter");
-    m_bNewData = true;
-    m_bCloned = true;
     return;
   }
   if (bHasFilter || !bFlateEncode) {
     m_pData = (uint8_t*)m_Acc.GetData();

[npm, 2016/11/28 20:31:02]

const_cast?

[Tom Sepez, 2016/11/28 21:38:08]

Done.

     m_dwSize = m_Acc.GetSize();
     m_pDict = pStream->GetDict();
     return;
   }
-  m_bNewData = true;
-  m_bCloned = true;
   // TODO(thestig): Move to Init() and check return value.
-  ::FlateEncode(m_Acc.GetData(), m_Acc.GetSize(), &m_pData, &m_dwSize);
-  m_pDict = ToDictionary(pStream->GetDict()->Clone().release());
+  uint8_t* buffer = nullptr;
+  ::FlateEncode(m_Acc.GetData(), m_Acc.GetSize(), &buffer, &m_dwSize);
+  m_pData = std::unique_ptr<uint8_t, FxFreeDeleter>(buffer);
+  m_pDict = ToDictionary(pStream->GetDict()->Clone());
   m_pDict->SetNewFor<CPDF_Number>("Length", static_cast<int>(m_dwSize));
   m_pDict->SetNewFor<CPDF_Name>("Filter", "FlateDecode");
   m_pDict->RemoveFor("DecodeParms");
 }
 
 CPDF_FlateEncoder::CPDF_FlateEncoder(const uint8_t* pBuffer,
                                      uint32_t size,
                                      bool bFlateEncode,
                                      bool bXRefStream)
-    : m_pData(nullptr),
-      m_dwSize(0),
-      m_pDict(nullptr),
-      m_bCloned(false),
-      m_bNewData(false) {
+    : m_dwSize(0) {
   if (!bFlateEncode) {
-    m_pData = (uint8_t*)pBuffer;
+    m_pData = const_cast<uint8_t*>(pBuffer);

[Wei Li, 2016/11/28 20:59:56]

Nit: is cast needed? pBuffer is of type "const uint8_t*"

[npm, 2016/11/28 21:01:29]

I think yes because m_pData is not const

[Wei Li, 2016/11/28 21:21:12]

Is const type better for m_pData?

[Tom Sepez, 2016/11/28 21:38:08]

We may free it later, so it's technically not const.

     m_dwSize = size;
     return;
   }
-  m_bNewData = true;
+  uint8_t* buffer = nullptr;
   // TODO(thestig): Move to Init() and check return value.
   if (bXRefStream)
-    ::PngEncode(pBuffer, size, &m_pData, &m_dwSize);
+    ::PngEncode(pBuffer, size, &buffer, &m_dwSize);
   else
-    ::FlateEncode(pBuffer, size, &m_pData, &m_dwSize);
+    ::FlateEncode(pBuffer, size, &buffer, &m_dwSize);
+  m_pData = std::unique_ptr<uint8_t, FxFreeDeleter>(buffer);
 }
 
-CPDF_FlateEncoder::~CPDF_FlateEncoder() {
-  if (m_bCloned)
-    delete m_pDict;
-  if (m_bNewData)
-    FX_Free(m_pData);
-}
+CPDF_FlateEncoder::~CPDF_FlateEncoder() {}
 
 class CPDF_Encryptor {
  public:
   CPDF_Encryptor(CPDF_CryptoHandler* pHandler,
                  int objnum,
                  uint8_t* src_data,
                  uint32_t src_size);
   ~CPDF_Encryptor();
 
   uint8_t* m_pData;
@@ skipping 88 matching lines @@
   }
   if (pFile->AppendString("/Length ") < 0) {
     return -1;
   }
   offset += len + 15;
 
   tempBuffer << m_Buffer;
   CPDF_FlateEncoder encoder(tempBuffer.GetBuffer(), tempBuffer.GetLength(),
                             true, false);
   CPDF_Encryptor encryptor(pCreator->m_pCryptoHandler, m_dwObjNum,
-                           encoder.m_pData, encoder.m_dwSize);
+                           encoder.m_pData.Get(), encoder.m_dwSize);
   if ((len = pFile->AppendDWord(encryptor.m_dwSize)) < 0) {
     return -1;
   }
   offset += len;
   if (pFile->AppendString("/Filter /FlateDecode") < 0) {
     return -1;
   }
   offset += 20;
   if ((len = pFile->AppendString(">>stream\r\n")) < 0) {
     return -1;
@@ skipping 229 matching lines @@
                                          dwEncryptObjNum, pFile)) < 0) {
         return false;
       }
       offset += len;
     }
   }
   if ((len = pFile->AppendString(">>stream\r\n")) < 0) {
     return false;
   }
   offset += len;
-  if (pFile->AppendBlock(encoder.m_pData, encoder.m_dwSize) < 0) {
+  if (pFile->AppendBlock(encoder.m_pData.Get(), encoder.m_dwSize) < 0) {

[npm, 2016/11/28 20:31:02]

Nit: no {}

[Tom Sepez, 2016/11/28 21:38:08]

Done.

     return false;
   }
   if ((len = pFile->AppendString("\r\nendstream\r\nendobj\r\n")) < 0) {
     return false;
   }
   offset += encoder.m_dwSize + len;
   m_PrevOffset = offset_tmp;
   return true;
 }
 bool CPDF_XRefStream::End(CPDF_Creator* pCreator, bool bEOF) {
@@ skipping 137 matching lines @@
   if (!m_pXRefStream->Start()) {
     return -1;
   }
   return 0;
 }
 int32_t CPDF_Creator::WriteStream(const CPDF_Object* pStream,
                                   uint32_t objnum,
                                   CPDF_CryptoHandler* pCrypto) {
   CPDF_FlateEncoder encoder(const_cast<CPDF_Stream*>(pStream->AsStream()),
                             pStream != m_pMetadata);
-  CPDF_Encryptor encryptor(pCrypto, objnum, encoder.m_pData, encoder.m_dwSize);
-  if ((uint32_t)encoder.m_pDict->GetIntegerFor("Length") !=
+  CPDF_Encryptor encryptor(pCrypto, objnum, encoder.m_pData.Get(),
+                           encoder.m_dwSize);
+  if (static_cast<uint32_t>(encoder.m_pDict->GetIntegerFor("Length")) !=
       encryptor.m_dwSize) {
     encoder.CloneDict();
     encoder.m_pDict->SetNewFor<CPDF_Number>(
         "Length", static_cast<int>(encryptor.m_dwSize));
   }
-  if (WriteDirectObj(objnum, encoder.m_pDict) < 0) {
+  if (WriteDirectObj(objnum, encoder.m_pDict.Get()) < 0)
     return -1;
-  }
+
   int len = m_File.AppendString("stream\r\n");
-  if (len < 0) {
+  if (len < 0)
     return -1;
-  }
+
   m_Offset += len;
-  if (m_File.AppendBlock(encryptor.m_pData, encryptor.m_dwSize) < 0) {
+  if (m_File.AppendBlock(encryptor.m_pData, encryptor.m_dwSize) < 0)
     return -1;
-  }
+
   m_Offset += encryptor.m_dwSize;
-  if ((len = m_File.AppendString("\r\nendstream")) < 0) {
+  if ((len = m_File.AppendString("\r\nendstream")) < 0)
     return -1;
-  }
+
   m_Offset += len;
   return 1;
 }
 int32_t CPDF_Creator::WriteIndirectObj(uint32_t objnum,
                                        const CPDF_Object* pObj) {
   int32_t len = m_File.AppendDWord(objnum);
   if (len < 0)
     return -1;
 
   m_Offset += len;
@@ skipping 71 matching lines @@
           bHex);
       if ((len = m_File.AppendString(content.AsStringC())) < 0) {
         return -1;
       }
       m_Offset += len;
       break;
     }
     case CPDF_Object::STREAM: {
       CPDF_FlateEncoder encoder(const_cast<CPDF_Stream*>(pObj->AsStream()),
                                 true);
-      CPDF_Encryptor encryptor(m_pCryptoHandler, objnum, encoder.m_pData,
+      CPDF_Encryptor encryptor(m_pCryptoHandler, objnum, encoder.m_pData.Get(),
                                encoder.m_dwSize);
-      if ((uint32_t)encoder.m_pDict->GetIntegerFor("Length") !=
+      if (static_cast<uint32_t>(encoder.m_pDict->GetIntegerFor("Length")) !=
           encryptor.m_dwSize) {
         encoder.CloneDict();
         encoder.m_pDict->SetNewFor<CPDF_Number>(
             "Length", static_cast<int>(encryptor.m_dwSize));
       }
-      if (WriteDirectObj(objnum, encoder.m_pDict) < 0) {
+      if (WriteDirectObj(objnum, encoder.m_pDict.Get()) < 0)
         return -1;
-      }
-      if ((len = m_File.AppendString("stream\r\n")) < 0) {
+
+      if ((len = m_File.AppendString("stream\r\n")) < 0)
         return -1;
-      }
+
       m_Offset += len;
-      if (m_File.AppendBlock(encryptor.m_pData, encryptor.m_dwSize) < 0) {
+      if (m_File.AppendBlock(encryptor.m_pData, encryptor.m_dwSize) < 0)
         return -1;
-      }
+
       m_Offset += encryptor.m_dwSize;
-      if ((len = m_File.AppendString("\r\nendstream")) < 0) {
+      if ((len = m_File.AppendString("\r\nendstream")) < 0)
         return -1;
-      }
+
       m_Offset += len;
       break;
     }
     case CPDF_Object::NAME: {
       if (m_File.AppendString("/") < 0) {
         return -1;
       }
       CFX_ByteString str = pObj->GetString();
       if ((len = m_File.AppendString(PDF_NameEncode(str).AsStringC())) < 0) {
         return -1;
@@ skipping 874 matching lines @@
   m_pCryptoHandler = nullptr;
 }
 void CPDF_Creator::ResetStandardSecurity() {
   if (!m_bLocalCryptoHandler)
     return;
 
   delete m_pCryptoHandler;
   m_pCryptoHandler = nullptr;
   m_bLocalCryptoHandler = false;
 }