Edit code cleanup

Minor cleans in the fpdfapi/edit code.

Change-Id: I7bb2f4add7c6e84f072501035b1f77c218174cca
Reviewed-on: https://pdfium-review.googlesource.com/5152
Reviewed-by: Nicolás Peña <npm@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: dsinclair <dsinclair@chromium.org>

4 files changed, 109 insertions, 82 deletions

diff --git a/core/fpdfapi/edit/cpdf_creator.cpp b/core/fpdfapi/edit/cpdf_creator.cpp
index d3cf5b85b7..5f540bd088 100644
--- a/core/fpdfapi/edit/cpdf_creator.cpp
+++ b/core/fpdfapi/edit/cpdf_creator.cpp
@@ -65,7 +65,6 @@ CPDF_Creator::CPDF_Creator(CPDF_Document* pDoc,
       m_dwEncryptObjNum(0),
       m_pCryptoHandler(m_pParser ? m_pParser->GetCryptoHandler() : nullptr),
       m_pMetadata(nullptr),
-      m_ObjectStreamSize(200),
       m_dwLastObjNum(m_pDocument->GetLastObjNum()),
       m_Offset(0),
       m_SavedOffset(0),
@@ -172,7 +171,7 @@ int32_t CPDF_Creator::WriteStream(const CPDF_Object* pStream,
     encoder.GetDict()->SetNewFor<CPDF_Number>(
         "Length", static_cast<int>(encryptor.GetSize()));
   }
-  if (WriteDirectObj(objnum, encoder.GetDict()) < 0)
+  if (WriteDirectObj(objnum, encoder.GetDict(), true) < 0)
     return -1;
 
   int len = m_File.AppendString("stream\r\n");
@@ -184,7 +183,8 @@ int32_t CPDF_Creator::WriteStream(const CPDF_Object* pStream,
     return -1;
 
   m_Offset += encryptor.GetSize();
-  if ((len = m_File.AppendString("\r\nendstream")) < 0)
+  len = m_File.AppendString("\r\nendstream");
+  if (len < 0)
     return -1;
 
   m_Offset += len;
@@ -198,7 +198,9 @@ int32_t CPDF_Creator::WriteIndirectObj(uint32_t objnum,
     return -1;
 
   m_Offset += len;
-  if ((len = m_File.AppendString(" 0 obj\r\n")) < 0)
+
+  len = m_File.AppendString(" 0 obj\r\n");
+  if (len < 0)
     return -1;
 
   m_Offset += len;
@@ -208,10 +210,12 @@ int32_t CPDF_Creator::WriteIndirectObj(uint32_t objnum,
     if (WriteStream(pObj, objnum, pHandler) < 0)
       return -1;
   } else {
-    if (WriteDirectObj(objnum, pObj) < 0)
+    if (WriteDirectObj(objnum, pObj, true) < 0)
       return -1;
   }
-  if ((len = m_File.AppendString("\r\nendobj\r\n")) < 0)
+
+  len = m_File.AppendString("\r\nendobj\r\n");
+  if (len < 0)
     return -1;
 
   m_Offset += len;
@@ -270,7 +274,7 @@ int32_t CPDF_Creator::WriteDirectObj(uint32_t objnum,
         encoder.GetDict()->SetNewFor<CPDF_Number>(
             "Length", static_cast<int>(encryptor.GetSize()));
       }
-      if (WriteDirectObj(objnum, encoder.GetDict()) < 0)
+      if (WriteDirectObj(objnum, encoder.GetDict(), true) < 0)
         return -1;
 
       int32_t len = m_File.AppendString("stream\r\n");
@@ -309,7 +313,7 @@ int32_t CPDF_Creator::WriteDirectObj(uint32_t objnum,
 
           m_Offset += len + 5;
         } else {
-          if (WriteDirectObj(objnum, pElement) < 0)
+          if (WriteDirectObj(objnum, pElement, true) < 0)
             return -1;
         }
       }
@@ -443,27 +447,20 @@ int32_t CPDF_Creator::WriteOldObjs() {
     int32_t iRet = WriteOldIndirectObject(objnum);
     if (iRet < 0)
       return iRet;
-    if (!iRet)
-      continue;
   }
   return 0;
 }
 
-int32_t CPDF_Creator::WriteNewObjs(bool bIncremental) {
-  uint32_t iCount = pdfium::CollectionSize<uint32_t>(m_NewObjNumArray);
-  uint32_t index = m_CurObjNum;
-  while (index < iCount) {
-    uint32_t objnum = m_NewObjNumArray[index];
+int32_t CPDF_Creator::WriteNewObjs() {
+  for (size_t i = m_CurObjNum; i < m_NewObjNumArray.size(); ++i) {
+    uint32_t objnum = m_NewObjNumArray[i];
     CPDF_Object* pObj = m_pDocument->GetIndirectObject(objnum);
-    if (!pObj) {
-      ++index;
+    if (!pObj)
       continue;
-    }
+
     m_ObjectOffsets[objnum] = m_Offset;
     if (WriteIndirectObj(pObj))
       return -1;
-
-    index++;
   }
   return 0;
 }
@@ -539,9 +536,10 @@ int32_t CPDF_Creator::WriteDoc_Stage1() {
       int32_t len = m_File.AppendDWord(version % 10);
       if (len < 0)
         return -1;
-
       m_Offset += len;
-      if ((len = m_File.AppendString("\r\n%\xA1\xB3\xC5\xD7\r\n")) < 0)
+
+      len = m_File.AppendString("\r\n%\xA1\xB3\xC5\xD7\r\n");
+      if (len < 0)
         return -1;
 
       m_Offset += len;
@@ -619,7 +617,7 @@ int32_t CPDF_Creator::WriteDoc_Stage2() {
     m_iStage = 26;
   }
   if (m_iStage == 26) {
-    int32_t iRet = WriteNewObjs(IsIncremental());
+    int32_t iRet = WriteNewObjs();
     if (iRet)
       return iRet;
 
@@ -927,10 +925,11 @@ bool CPDF_Creator::Create(uint32_t flags) {
   return Continue() > -1;
 }
 
-void CPDF_Creator::InitID(bool bDefault) {
+void CPDF_Creator::InitID() {
   CPDF_Array* pOldIDArray = m_pParser ? m_pParser->GetIDArray() : nullptr;
-  bool bNewId = !m_pIDArray;
-  if (bNewId) {
+
+  bool idArrayPreExisting = !!m_pIDArray;
+  if (!idArrayPreExisting) {
     m_pIDArray = pdfium::MakeUnique<CPDF_Array>();
     CPDF_Object* pID1 = pOldIDArray ? pOldIDArray->GetObjectAt(0) : nullptr;
     if (pID1) {
@@ -942,8 +941,6 @@ void CPDF_Creator::InitID(bool bDefault) {
       m_pIDArray->AddNew<CPDF_String>(bsBuffer, true);
     }
   }
-  if (!bDefault)
-    return;
 
   if (pOldIDArray) {
     CPDF_Object* pID2 = pOldIDArray->GetObjectAt(1);
@@ -959,7 +956,7 @@ void CPDF_Creator::InitID(bool bDefault) {
   }
 
   m_pIDArray->Add(m_pIDArray->GetObjectAt(0)->Clone());
-  if (m_pEncryptDict && !pOldIDArray && m_pParser && bNewId) {
+  if (m_pEncryptDict && !pOldIDArray && m_pParser && !idArrayPreExisting) {
     if (m_pEncryptDict->GetStringFor("Filter") == "Standard") {
       CFX_ByteString user_pass = m_pParser->GetPassword();
       uint32_t flag = PDF_ENCRYPT_CONTENT;
@@ -1003,7 +1000,6 @@ int32_t CPDF_Creator::Continue() {
 bool CPDF_Creator::SetFileVersion(int32_t fileVersion) {
   if (fileVersion < 10 || fileVersion > 17)
     return false;
-
   m_FileVersion = fileVersion;
   return true;
 }
diff --git a/core/fpdfapi/edit/cpdf_creator.h b/core/fpdfapi/edit/cpdf_creator.h
index e263c6cc1d..bda6507f21 100644
--- a/core/fpdfapi/edit/cpdf_creator.h
+++ b/core/fpdfapi/edit/cpdf_creator.h
@@ -34,9 +34,9 @@ class CPDF_Creator {
   ~CPDF_Creator();
 
   void RemoveSecurity();
-  bool Create(uint32_t flags = 0);
+  bool Create(uint32_t flags);
   int32_t Continue();
-  bool SetFileVersion(int32_t fileVersion = 17);
+  bool SetFileVersion(int32_t fileVersion);
 
   CFX_FileBufferArchive* GetFile() { return &m_File; }
 
@@ -44,7 +44,6 @@ class CPDF_Creator {
   void IncrementOffset(FX_FILESIZE inc);
   uint32_t GetNextObjectNumber() { return ++m_dwLastObjNum; }
   uint32_t GetLastObjectNumber() const { return m_dwLastObjNum; }
-  int32_t GetObjectStreamSize() const { return m_ObjectStreamSize; }
   CPDF_CryptoHandler* GetCryptoHandler() { return m_pCryptoHandler.Get(); }
   CPDF_Document* GetDocument() const { return m_pDocument; }
   CPDF_Array* GetIDArray() const { return m_pIDArray.get(); }
@@ -65,7 +64,7 @@ class CPDF_Creator {
 
   void InitOldObjNumOffsets();
   void InitNewObjNumOffsets();
-  void InitID(bool bDefault = true);
+  void InitID();
 
   int32_t AppendObjectNumberToXRef(uint32_t objnum);
 
@@ -76,11 +75,11 @@ class CPDF_Creator {
 
   int32_t WriteOldIndirectObject(uint32_t objnum);
   int32_t WriteOldObjs();
-  int32_t WriteNewObjs(bool bIncremental);
+  int32_t WriteNewObjs();
   int32_t WriteIndirectObj(const CPDF_Object* pObj);
   int32_t WriteDirectObj(uint32_t objnum,
                          const CPDF_Object* pObj,
-                         bool bEncrypt = true);
+                         bool bEncrypt);
   int32_t WriteIndirectObjectToStream(const CPDF_Object* pObj);
   int32_t WriteIndirectObj(uint32_t objnum, const CPDF_Object* pObj);
   int32_t WriteIndirectObjectToStream(uint32_t objnum,
@@ -101,7 +100,6 @@ class CPDF_Creator {
   CFX_RetainPtr<CPDF_CryptoHandler> m_pCryptoHandler;
   CPDF_Object* m_pMetadata;
   std::unique_ptr<CPDF_XRefStream> m_pXRefStream;
-  int32_t m_ObjectStreamSize;
   uint32_t m_dwLastObjNum;
   CFX_FileBufferArchive m_File;
   FX_FILESIZE m_Offset;
diff --git a/core/fpdfapi/edit/cpdf_objectstream.cpp b/core/fpdfapi/edit/cpdf_objectstream.cpp
index e3f6e4c8e9..fa0284433b 100644
--- a/core/fpdfapi/edit/cpdf_objectstream.cpp
+++ b/core/fpdfapi/edit/cpdf_objectstream.cpp
@@ -47,6 +47,7 @@ void CPDF_ObjectStream::CompressIndirectObject(uint32_t dwObjNum,
 
 FX_FILESIZE CPDF_ObjectStream::End(CPDF_Creator* pCreator) {
   ASSERT(pCreator);
+
   if (m_Items.empty())
     return 0;
 
@@ -62,32 +63,38 @@ FX_FILESIZE CPDF_ObjectStream::End(CPDF_Creator* pCreator) {
   int32_t len = pFile->AppendDWord(m_dwObjNum);
   if (len < 0)
     return -1;
-
   pCreator->IncrementOffset(len);
-  if ((len = pFile->AppendString(" 0 obj\r\n<</Type /ObjStm /N ")) < 0)
+
+  len = pFile->AppendString(" 0 obj\r\n<</Type /ObjStm /N ");
+  if (len < 0)
     return -1;
 
   pCreator->IncrementOffset(len);
   uint32_t iCount = pdfium::CollectionSize<uint32_t>(m_Items);
-  if ((len = pFile->AppendDWord(iCount)) < 0)
+  len = pFile->AppendDWord(iCount);
+  if (len < 0)
     return -1;
-
   pCreator->IncrementOffset(len);
+
   if (pFile->AppendString("/First ") < 0)
     return -1;
-  if ((len = pFile->AppendDWord((uint32_t)tempBuffer.GetLength())) < 0)
+
+  len = pFile->AppendDWord(static_cast<uint32_t>(tempBuffer.GetLength()));
+  if (len < 0)
     return -1;
   if (pFile->AppendString("/Length ") < 0)
     return -1;
-
   pCreator->IncrementOffset(len + 15);
 
   tempBuffer << m_Buffer;
+
   CPDF_FlateEncoder encoder(tempBuffer.GetBuffer(), tempBuffer.GetLength(),
                             true, false);
   CPDF_Encryptor encryptor(pCreator->GetCryptoHandler(), m_dwObjNum,
                            encoder.GetData(), encoder.GetSize());
-  if ((len = pFile->AppendDWord(encryptor.GetSize())) < 0)
+
+  len = pFile->AppendDWord(encryptor.GetSize());
+  if (len < 0)
     return -1;
 
   pCreator->IncrementOffset(len);
@@ -95,13 +102,15 @@ FX_FILESIZE CPDF_ObjectStream::End(CPDF_Creator* pCreator) {
     return -1;
 
   pCreator->IncrementOffset(20);
-  if ((len = pFile->AppendString(">>stream\r\n")) < 0)
+  len = pFile->AppendString(">>stream\r\n");
+  if (len < 0)
     return -1;
   if (pFile->AppendBlock(encryptor.GetData(), encryptor.GetSize()) < 0)
     return -1;
 
   pCreator->IncrementOffset(len + encryptor.GetSize());
-  if ((len = pFile->AppendString("\r\nendstream\r\nendobj\r\n")) < 0)
+  len = pFile->AppendString("\r\nendstream\r\nendobj\r\n");
+  if (len < 0)
     return -1;
 
   pCreator->IncrementOffset(len);
diff --git a/core/fpdfapi/edit/cpdf_xrefstream.cpp b/core/fpdfapi/edit/cpdf_xrefstream.cpp
index 9ec27f291c..49174a54bc 100644
--- a/core/fpdfapi/edit/cpdf_xrefstream.cpp
+++ b/core/fpdfapi/edit/cpdf_xrefstream.cpp
@@ -15,6 +15,8 @@
 
 namespace {
 
+const int32_t kObjectStreamMaxSize = 200;
+
 int32_t WriteTrailer(CPDF_Document* pDocument,
                      CFX_FileBufferArchive* pFile,
                      CPDF_Array* pIDArray) {
@@ -35,14 +37,18 @@ int32_t WriteTrailer(CPDF_Document* pDocument,
         continue;
       if (pFile->AppendString(("/")) < 0)
         return -1;
-      if ((len = pFile->AppendString(PDF_NameEncode(key).AsStringC())) < 0)
+
+      len = pFile->AppendString(PDF_NameEncode(key).AsStringC());
+      if (len < 0)
         return -1;
 
       offset += len + 1;
       if (!pValue->IsInline()) {
         if (pFile->AppendString(" ") < 0)
           return -1;
-        if ((len = pFile->AppendDWord(pValue->GetObjNum())) < 0)
+
+        len = pFile->AppendDWord(pValue->GetObjNum());
+        if (len < 0)
           return -1;
         if (pFile->AppendString(" 0 R ") < 0)
           return -1;
@@ -65,7 +71,9 @@ int32_t WriteTrailer(CPDF_Document* pDocument,
   }
   if (pFile->AppendString("\r\n/Root ") < 0)
     return -1;
-  if ((len = pFile->AppendDWord(pDocument->GetRoot()->GetObjNum())) < 0)
+
+  len = pFile->AppendDWord(pDocument->GetRoot()->GetObjNum());
+  if (len < 0)
     return -1;
   if (pFile->AppendString(" 0 R\r\n") < 0)
     return -1;
@@ -74,7 +82,9 @@ int32_t WriteTrailer(CPDF_Document* pDocument,
   if (pDocument->GetInfo()) {
     if (pFile->AppendString("/Info ") < 0)
       return -1;
-    if ((len = pFile->AppendDWord(pDocument->GetInfo()->GetObjNum())) < 0)
+
+    len = pFile->AppendDWord(pDocument->GetInfo()->GetObjNum());
+    if (len < 0)
       return -1;
     if (pFile->AppendString(" 0 R\r\n") < 0)
       return -1;
@@ -104,7 +114,9 @@ int32_t WriteEncryptDictObjectReference(uint32_t dwObjNum,
   offset += 8;
   if (pFile->AppendString(" ") < 0)
     return -1;
-  if ((len = pFile->AppendDWord(dwObjNum)) < 0)
+
+  len = pFile->AppendDWord(dwObjNum);
+  if (len < 0)
     return -1;
   if (pFile->AppendString(" 0 R ") < 0)
     return -1;
@@ -165,7 +177,7 @@ int32_t CPDF_XRefStream::CompressIndirectObject(uint32_t dwObjNum,
   ASSERT(pCreator);
 
   m_ObjStream.CompressIndirectObject(dwObjNum, pObj);
-  if (m_ObjStream.ItemCount() < pCreator->GetObjectStreamSize() &&
+  if (m_ObjStream.ItemCount() < kObjectStreamMaxSize &&
       m_ObjStream.IsNotFull()) {
     return 1;
   }
@@ -179,7 +191,7 @@ int32_t CPDF_XRefStream::CompressIndirectObject(uint32_t dwObjNum,
   ASSERT(pCreator);
 
   m_ObjStream.CompressIndirectObject(dwObjNum, pBuffer, dwSize);
-  if (m_ObjStream.ItemCount() < pCreator->GetObjectStreamSize() &&
+  if (m_ObjStream.ItemCount() < kObjectStreamMaxSize &&
       m_ObjStream.IsNotFull()) {
     return 1;
   }
@@ -227,6 +239,7 @@ int32_t CPDF_XRefStream::EndObjectStream(CPDF_Creator* pCreator, bool bEOF) {
 
     return 1;
   }
+
   for (auto it = m_IndexArray.begin() + m_iSeg; it != m_IndexArray.end();
        ++it) {
     for (uint32_t m = it->objnum; m < it->objnum + it->count; ++m) {
@@ -272,46 +285,51 @@ bool CPDF_XRefStream::GenerateXRefStream(CPDF_Creator* pCreator, bool bEOF) {
   int32_t len = pFile->AppendDWord(objnum);
   if (len < 0)
     return false;
-
   pCreator->IncrementOffset(len);
-  if ((len = pFile->AppendString(" 0 obj\r\n<</Type /XRef/W[1 4 2]/Index[")) <
-      0) {
+
+  len = pFile->AppendString(" 0 obj\r\n<</Type /XRef/W[1 4 2]/Index[");
+  if (len < 0)
     return false;
-  }
   pCreator->IncrementOffset(len);
+
   if (!pCreator->IsIncremental()) {
-    if ((len = pFile->AppendDWord(0)) < 0)
-      return false;
-    if ((len = pFile->AppendString(" ")) < 0)
+    if (pFile->AppendDWord(0) < 0)
       return false;
 
-    pCreator->IncrementOffset(len + 1);
-    if ((len = pFile->AppendDWord(objnum + 1)) < 0)
+    len = pFile->AppendString(" ");
+    if (len < 0)
       return false;
+    pCreator->IncrementOffset(len + 1);
 
+    len = pFile->AppendDWord(objnum + 1);
+    if (len < 0)
+      return false;
     pCreator->IncrementOffset(len);
   } else {
     for (const auto& pair : m_IndexArray) {
-      if ((len = pFile->AppendDWord(pair.objnum)) < 0)
+      len = pFile->AppendDWord(pair.objnum);
+      if (len < 0)
         return false;
       if (pFile->AppendString(" ") < 0)
         return false;
-
       pCreator->IncrementOffset(len + 1);
-      if ((len = pFile->AppendDWord(pair.count)) < 0)
+
+      len = pFile->AppendDWord(pair.count);
+      if (len < 0)
         return false;
       if (pFile->AppendString(" ") < 0)
         return false;
-
       pCreator->IncrementOffset(len + 1);
     }
   }
   if (pFile->AppendString("]/Size ") < 0)
     return false;
-  if ((len = pFile->AppendDWord(objnum + 1)) < 0)
-    return false;
 
+  len = pFile->AppendDWord(objnum + 1);
+  if (len < 0)
+    return false;
   pCreator->IncrementOffset(len + 7);
+
   if (m_PrevOffset > 0) {
     if (pFile->AppendString("/Prev ") < 0)
       return false;
@@ -330,46 +348,52 @@ bool CPDF_XRefStream::GenerateXRefStream(CPDF_Creator* pCreator, bool bEOF) {
                             true);
   if (pFile->AppendString("/Filter /FlateDecode") < 0)
     return false;
-
   pCreator->IncrementOffset(20);
-  if ((len = pFile->AppendString("/DecodeParms<</Columns 7/Predictor 12>>")) <
-      0) {
-    return false;
-  }
 
+  len = pFile->AppendString("/DecodeParms<</Columns 7/Predictor 12>>");
+  if (len < 0)
+    return false;
   pCreator->IncrementOffset(len);
+
   if (pFile->AppendString("/Length ") < 0)
     return false;
-  if ((len = pFile->AppendDWord(encoder.GetSize())) < 0)
+
+  len = pFile->AppendDWord(encoder.GetSize());
+  if (len < 0)
     return false;
 
   pCreator->IncrementOffset(len + 8);
   if (bEOF) {
-    if ((len = WriteTrailer(pCreator->GetDocument(), pFile,
-                            pCreator->GetIDArray())) < 0) {
+    len = WriteTrailer(pCreator->GetDocument(), pFile, pCreator->GetIDArray());
+    if (len < 0)
       return false;
-    }
     pCreator->IncrementOffset(len);
 
     if (CPDF_Dictionary* encryptDict = pCreator->GetEncryptDict()) {
       uint32_t dwEncryptObjNum = encryptDict->GetObjNum();
       if (dwEncryptObjNum == 0)
         dwEncryptObjNum = pCreator->GetEncryptObjectNumber();
-      if ((len = WriteEncryptDictObjectReference(dwEncryptObjNum, pFile)) < 0)
+
+      len = WriteEncryptDictObjectReference(dwEncryptObjNum, pFile);
+      if (len < 0)
         return false;
       pCreator->IncrementOffset(len);
     }
   }
-  if ((len = pFile->AppendString(">>stream\r\n")) < 0)
-    return false;
 
+  len = pFile->AppendString(">>stream\r\n");
+  if (len < 0)
+    return false;
   pCreator->IncrementOffset(len);
+
   if (pFile->AppendBlock(encoder.GetData(), encoder.GetSize()) < 0)
     return false;
-  if ((len = pFile->AppendString("\r\nendstream\r\nendobj\r\n")) < 0)
-    return false;
 
+  len = pFile->AppendString("\r\nendstream\r\nendobj\r\n");
+  if (len < 0)
+    return false;
   pCreator->IncrementOffset(encoder.GetSize() + len);
+
   m_PrevOffset = offset_tmp;
   return true;
 }