Make CPDF_IndirectObjectHolder::InsertIndirectObject return a bool.

Fix some nits in CPDF_Parser::LoadCrossRefV5() and other callers.

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

4 files changed, 38 insertions, 39 deletions

diff --git a/core/fpdfapi/fpdf_parser/cfdf_document.cpp b/core/fpdfapi/fpdf_parser/cfdf_document.cpp
index 3039c32938..d4f49f659b 100644
--- a/core/fpdfapi/fpdf_parser/cfdf_document.cpp
+++ b/core/fpdfapi/fpdf_parser/cfdf_document.cpp
@@ -28,22 +28,21 @@ CFDF_Document* CFDF_Document::CreateNewDoc() {
   pDoc->m_pRootDict->SetAt("FDF", pFDFDict);
   return pDoc;
 }
+
 CFDF_Document* CFDF_Document::ParseFile(IFX_FileRead* pFile, FX_BOOL bOwnFile) {
-  if (!pFile) {
-    return NULL;
-  }
-  CFDF_Document* pDoc = new CFDF_Document;
+  if (!pFile)
+    return nullptr;
+
+  std::unique_ptr<CFDF_Document> pDoc(new CFDF_Document);
   pDoc->ParseStream(pFile, bOwnFile);
-  if (!pDoc->m_pRootDict) {
-    delete pDoc;
-    return NULL;
-  }
-  return pDoc;
+  return pDoc->m_pRootDict ? pDoc.release() : nullptr;
 }
+
 CFDF_Document* CFDF_Document::ParseMemory(const uint8_t* pData, uint32_t size) {
   return CFDF_Document::ParseFile(FX_CreateMemoryStream((uint8_t*)pData, size),
                                   TRUE);
 }
+
 void CFDF_Document::ParseStream(IFX_FileRead* pFile, FX_BOOL bOwnFile) {
   m_pFile = pFile;
   m_bOwnFile = bOwnFile;
@@ -55,26 +54,25 @@ void CFDF_Document::ParseStream(IFX_FileRead* pFile, FX_BOOL bOwnFile) {
     if (bNumber) {
       uint32_t objnum = FXSYS_atoui(word.c_str());
       word = parser.GetNextWord(&bNumber);
-      if (!bNumber) {
+      if (!bNumber)
         break;
-      }
+
       word = parser.GetNextWord(nullptr);
-      if (word != "obj") {
+      if (word != "obj")
         break;
-      }
+
       CPDF_Object* pObj = parser.GetObject(this, objnum, 0, true);
-      if (!pObj) {
+      if (!pObj)
         break;
-      }
+
       InsertIndirectObject(objnum, pObj);
       word = parser.GetNextWord(nullptr);
-      if (word != "endobj") {
+      if (word != "endobj")
         break;
-      }
     } else {
-      if (word != "trailer") {
+      if (word != "trailer")
         break;
-      }
+
       if (CPDF_Dictionary* pMainDict =
               ToDictionary(parser.GetObject(this, 0, 0, true))) {
         m_pRootDict = pMainDict->GetDictBy("Root");
@@ -84,6 +82,7 @@ void CFDF_Document::ParseStream(IFX_FileRead* pFile, FX_BOOL bOwnFile) {
     }
   }
 }
+
 FX_BOOL CFDF_Document::WriteBuf(CFX_ByteTextBuf& buf) const {
   if (!m_pRootDict) {
     return FALSE;
diff --git a/core/fpdfapi/fpdf_parser/cpdf_indirect_object_holder.cpp b/core/fpdfapi/fpdf_parser/cpdf_indirect_object_holder.cpp
index ef3395d3ae..33f45a53bf 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_indirect_object_holder.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_indirect_object_holder.cpp
@@ -65,20 +65,21 @@ void CPDF_IndirectObjectHolder::ReleaseIndirectObject(uint32_t objnum) {
   m_IndirectObjs.erase(it);
 }
 
-FX_BOOL CPDF_IndirectObjectHolder::InsertIndirectObject(uint32_t objnum,
-                                                        CPDF_Object* pObj) {
+bool CPDF_IndirectObjectHolder::InsertIndirectObject(uint32_t objnum,
+                                                     CPDF_Object* pObj) {
   if (!objnum || !pObj)
-    return FALSE;
+    return false;
+
   auto it = m_IndirectObjs.find(objnum);
   if (it != m_IndirectObjs.end()) {
     if (pObj->GetGenNum() <= it->second->GetGenNum()) {
       pObj->Destroy();
-      return FALSE;
+      return false;
     }
     it->second->Destroy();
   }
   pObj->m_ObjNum = objnum;
   m_IndirectObjs[objnum] = pObj;
   m_LastObjNum = std::max(m_LastObjNum, objnum);
-  return TRUE;
+  return true;
 }
diff --git a/core/fpdfapi/fpdf_parser/cpdf_parser.cpp b/core/fpdfapi/fpdf_parser/cpdf_parser.cpp
index a6b99e5ddb..5540877642 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_parser.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_parser.cpp
@@ -979,16 +979,13 @@ FX_BOOL CPDF_Parser::LoadCrossRefV5(FX_FILESIZE* pos, FX_BOOL bMainXRef) {
     return FALSE;
 
   if (m_pDocument) {
-    FX_BOOL bInserted = FALSE;
-    CPDF_Dictionary* pDict = m_pDocument->GetRoot();
-    if (!pDict || pDict->GetObjNum() != pObject->m_ObjNum) {
-      bInserted = m_pDocument->InsertIndirectObject(pObject->m_ObjNum, pObject);
-    } else {
+    CPDF_Dictionary* pRootDict = m_pDocument->GetRoot();
+    if (pRootDict && pRootDict->GetObjNum() == pObject->m_ObjNum) {
       if (pObject->IsStream())
         pObject->Release();
+      return FALSE;
     }
-
-    if (!bInserted)
+    if (!m_pDocument->InsertIndirectObject(pObject->m_ObjNum, pObject))
       return FALSE;
   }
 
@@ -996,24 +993,26 @@ FX_BOOL CPDF_Parser::LoadCrossRefV5(FX_FILESIZE* pos, FX_BOOL bMainXRef) {
   if (!pStream)
     return FALSE;
 
-  *pos = pStream->GetDict()->GetIntegerBy("Prev");
-  int32_t size = pStream->GetDict()->GetIntegerBy("Size");
+  CPDF_Dictionary* pDict = pStream->GetDict();
+  *pos = pDict->GetIntegerBy("Prev");
+  int32_t size = pDict->GetIntegerBy("Size");
   if (size < 0) {
     pStream->Release();
     return FALSE;
   }
 
+  CPDF_Dictionary* pNewTrailer = ToDictionary(pDict->Clone());
   if (bMainXRef) {
-    m_pTrailer = ToDictionary(pStream->GetDict()->Clone());
+    m_pTrailer = pNewTrailer;
     ShrinkObjectMap(size);
     for (auto& it : m_ObjectInfo)
       it.second.type = 0;
   } else {
-    m_Trailers.Add(ToDictionary(pStream->GetDict()->Clone()));
+    m_Trailers.Add(pNewTrailer);
   }
 
   std::vector<std::pair<int32_t, int32_t>> arrIndex;
-  CPDF_Array* pArray = pStream->GetDict()->GetArrayBy("Index");
+  CPDF_Array* pArray = pDict->GetArrayBy("Index");
   if (pArray) {
     for (size_t i = 0; i < pArray->GetCount() / 2; i++) {
       CPDF_Object* pStartNumObj = pArray->GetObjectAt(i * 2);
@@ -1031,7 +1030,7 @@ FX_BOOL CPDF_Parser::LoadCrossRefV5(FX_FILESIZE* pos, FX_BOOL bMainXRef) {
   if (arrIndex.size() == 0)
     arrIndex.push_back(std::make_pair(0, size));
 
-  pArray = pStream->GetDict()->GetArrayBy("W");
+  pArray = pDict->GetArrayBy("W");
   if (!pArray) {
     pStream->Release();
     return FALSE;
@@ -1039,7 +1038,7 @@ FX_BOOL CPDF_Parser::LoadCrossRefV5(FX_FILESIZE* pos, FX_BOOL bMainXRef) {
 
   CFX_ArrayTemplate<uint32_t> WidthArray;
   FX_SAFE_UINT32 dwAccWidth = 0;
-  for (size_t i = 0; i < pArray->GetCount(); i++) {
+  for (size_t i = 0; i < pArray->GetCount(); ++i) {
     WidthArray.Add(pArray->GetIntegerAt(i));
     dwAccWidth += WidthArray[i];
   }
diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_indirect_object_holder.h b/core/fpdfapi/fpdf_parser/include/cpdf_indirect_object_holder.h
index a219181179..c29746fdd7 100644
--- a/core/fpdfapi/fpdf_parser/include/cpdf_indirect_object_holder.h
+++ b/core/fpdfapi/fpdf_parser/include/cpdf_indirect_object_holder.h
@@ -27,7 +27,7 @@ class CPDF_IndirectObjectHolder {
   void ReleaseIndirectObject(uint32_t objnum);
 
   // Takes ownership of |pObj|.
-  FX_BOOL InsertIndirectObject(uint32_t objnum, CPDF_Object* pObj);
+  bool InsertIndirectObject(uint32_t objnum, CPDF_Object* pObj);
 
   uint32_t GetLastObjNum() const { return m_LastObjNum; }
   iterator begin() { return m_IndirectObjs.begin(); }