Return unique_ptr from CPDF_Object::Clone().

fpdfsdk/fpdfppo.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 "public/fpdf_ppo.h"
 
 #include <map>
 #include <memory>
@@ skipping 91 matching lines @@
     if (!pSrcPageDict || !pCurPageDict)
       return false;
 
     // Clone the page dictionary
     for (const auto& it : *pSrcPageDict) {
       const CFX_ByteString& cbSrcKeyStr = it.first;
       CPDF_Object* pObj = it.second;
       if (cbSrcKeyStr.Compare(("Type")) && cbSrcKeyStr.Compare(("Parent"))) {
         if (pCurPageDict->KeyExist(cbSrcKeyStr))
           pCurPageDict->RemoveFor(cbSrcKeyStr);
-        pCurPageDict->SetFor(cbSrcKeyStr, pObj->Clone());
+        pCurPageDict->SetFor(cbSrcKeyStr, pObj->Clone().release());
       }
     }
 
     // inheritable item
     CPDF_Object* pInheritable = nullptr;
     // 1 MediaBox  //required
     if (!pCurPageDict->KeyExist("MediaBox")) {
       pInheritable = PageDictGetInheritableTag(pSrcPageDict, "MediaBox");
       if (!pInheritable) {
         // Search the "CropBox" from source page dictionary,
         // if not exists,we take the letter size.
         pInheritable = PageDictGetInheritableTag(pSrcPageDict, "CropBox");
         if (pInheritable) {
-          pCurPageDict->SetFor("MediaBox", pInheritable->Clone());
+          pCurPageDict->SetFor("MediaBox", pInheritable->Clone().release());
         } else {
           // Make the default size to be letter size (8.5'x11')
           CPDF_Array* pArray = new CPDF_Array;
           pArray->AddNumber(0);
           pArray->AddNumber(0);
           pArray->AddNumber(612);
           pArray->AddNumber(792);
           pCurPageDict->SetFor("MediaBox", pArray);
         }
       } else {
-        pCurPageDict->SetFor("MediaBox", pInheritable->Clone());
+        pCurPageDict->SetFor("MediaBox", pInheritable->Clone().release());
       }
     }
     // 2 Resources //required
     if (!pCurPageDict->KeyExist("Resources")) {
       pInheritable = PageDictGetInheritableTag(pSrcPageDict, "Resources");
       if (!pInheritable)
         return false;
-      pCurPageDict->SetFor("Resources", pInheritable->Clone());
+      pCurPageDict->SetFor("Resources", pInheritable->Clone().release());
     }
     // 3 CropBox  //Optional
     if (!pCurPageDict->KeyExist("CropBox")) {
       pInheritable = PageDictGetInheritableTag(pSrcPageDict, "CropBox");
       if (pInheritable)
-        pCurPageDict->SetFor("CropBox", pInheritable->Clone());
+        pCurPageDict->SetFor("CropBox", pInheritable->Clone().release());
     }
     // 4 Rotate  //Optional
     if (!pCurPageDict->KeyExist("Rotate")) {
       pInheritable = PageDictGetInheritableTag(pSrcPageDict, "Rotate");
       if (pInheritable)
-        pCurPageDict->SetFor("Rotate", pInheritable->Clone());
+        pCurPageDict->SetFor("Rotate", pInheritable->Clone().release());
     }
 
     // Update the reference
     uint32_t dwOldPageObj = pSrcPageDict->GetObjNum();
     uint32_t dwNewPageObj = pCurPageDict->GetObjNum();
-
     (*pObjNumberMap)[dwOldPageObj] = dwNewPageObj;
-
     UpdateReference(pCurPageDict, pDestPDFDoc, pObjNumberMap.get());
     ++curpage;
   }
 
   return true;
 }
 
 CPDF_Object* CPDF_PageOrganizer::PageDictGetInheritableTag(
     CPDF_Dictionary* pDict,
     const CFX_ByteString& bsSrcTag) {
@@ skipping 94 matching lines @@
   const auto it = pObjNumberMap->find(dwObjnum);
   if (it != pObjNumberMap->end())
     dwNewObjNum = it->second;
   if (dwNewObjNum)
     return dwNewObjNum;
 
   CPDF_Object* pDirect = pRef->GetDirect();
   if (!pDirect)
     return 0;
 
-  CPDF_Object* pClone = pDirect->Clone();
+  std::unique_ptr<CPDF_Object> pClone = pDirect->Clone();
   if (!pClone)

[Lei Zhang, 2016/11/08 18:55:43]

BTW, there's a few places where we check for cloning failures, but I don't think
that can happen.

[Tom Sepez, 2016/11/09 18:20:46]

There's a few places where we return nullptr if we are caught in a cycle, but
that would imply that we've already got some top-level thing, so I think you're
right.  Fixing.

     return 0;
 
   if (CPDF_Dictionary* pDictClone = pClone->AsDictionary()) {
     if (pDictClone->KeyExist("Type")) {
       CFX_ByteString strType = pDictClone->GetStringFor("Type");
-      if (!FXSYS_stricmp(strType.c_str(), "Pages")) {
-        delete pDictClone;
+      if (!FXSYS_stricmp(strType.c_str(), "Pages"))
         return 4;
-      }
-      if (!FXSYS_stricmp(strType.c_str(), "Page")) {
-        delete pDictClone;
+      if (!FXSYS_stricmp(strType.c_str(), "Page"))
         return 0;
-      }
     }
   }
-  dwNewObjNum = pDoc->AddIndirectObject(pClone);
+  dwNewObjNum = pDoc->AddIndirectObject(pClone.get());
   (*pObjNumberMap)[dwObjnum] = dwNewObjNum;
-  if (!UpdateReference(pClone, pDoc, pObjNumberMap)) {
-    delete pClone;
+  if (!UpdateReference(pClone.get(), pDoc, pObjNumberMap))
     return 0;
-  }
+
+  pClone.release();  // TODO(tsepez): figure out ownership.

[Lei Zhang, 2016/11/08 18:55:43]

On line 291 , AddIndirectObjet() is suppose to take it?

[Tom Sepez, 2016/11/09 18:20:46]

Yup.

   return dwNewObjNum;
 }
 
 FPDF_BOOL ParserPageRangeString(CFX_ByteString rangstring,
                                 std::vector<uint16_t>* pageArray,
                                 int nCount) {
   if (rangstring.GetLength() != 0) {
     rangstring.Remove(' ');
     int nLength = rangstring.GetLength();
     CFX_ByteString cbCompareString("0123456789-,");
@@ skipping 80 matching lines @@
 
   CPDF_Dictionary* pSrcDict = pSrcDoc->GetRoot();
   pSrcDict = pSrcDict->GetDictFor("ViewerPreferences");
   if (!pSrcDict)
     return false;
 
   CPDF_Dictionary* pDstDict = pDstDoc->GetRoot();
   if (!pDstDict)
     return false;
 
-  pDstDict->SetFor("ViewerPreferences", pSrcDict->CloneDirectObject());
+  pDstDict->SetFor("ViewerPreferences",
+                   pSrcDict->CloneDirectObject().release());
   return true;
 }