Remove some checks for object creation failures. They cannot fail.

core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.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 <set>
 #include <utility>
 #include <vector>
 
 #include "../../../../third_party/base/nonstd_unique_ptr.h"
 #include "../../../include/fpdfapi/fpdf_module.h"
 #include "../../../include/fpdfapi/fpdf_page.h"
 #include "../../../include/fpdfapi/fpdf_parser.h"
 #include "../../../include/fxcrt/fx_safe_types.h"
 #include "../fpdf_page/pageint.h"
 
 namespace {
 
+struct SearchTagRecord {
+  const uint8_t* m_pTag;
+  FX_DWORD m_Len;
+  FX_DWORD m_Offset;
+};
+
 int CompareFileSize(const void* p1, const void* p2) {
   return *(FX_FILESIZE*)p1 - *(FX_FILESIZE*)p2;
 }
 
 int32_t GetHeaderOffset(IFX_FileRead* pFile) {
   const FX_DWORD tag = FXDWORD_FROM_LSBFIRST(0x46445025);
   const size_t kBufSize = 4;
   uint8_t buf[kBufSize];
   int32_t offset = 0;
   while (offset <= 1024) {
@@ skipping 22 matching lines @@
   return !pObj || pObj->GetType() == iType;
 }
 
 FX_DWORD GetVarInt(const uint8_t* p, int32_t n) {
   FX_DWORD result = 0;
   for (int32_t i = 0; i < n; ++i)
     result = result * 256 + p[i];
   return result;
 }
 
+int32_t GetStreamNCount(CPDF_StreamAcc* pObjStream) {
+  return pObjStream->GetDict()->GetInteger(FX_BSTRC("N"));

[Tom Sepez, 2015/10/09 16:47:32]

I don't think we need the BSTRC() macros, really, but I'm not 100%.  The ctor
for the CFX_ string type will call strlen() on the literal, but I thought that
all our compilers were smart enough to fix that. Something for another day.

[Lei Zhang, 2015/10/19 08:05:51]

Acknowledged.

+}
+
+int32_t GetStreamFirst(CPDF_StreamAcc* pObjStream) {
+  return pObjStream->GetDict()->GetInteger(FX_BSTRC("First"));
+}
+
 }  // namespace
 
 FX_BOOL IsSignatureDict(const CPDF_Dictionary* pDict) {
   CPDF_Object* pType = pDict->GetElementValue(FX_BSTRC("Type"));
   if (!pType) {
     pType = pDict->GetElementValue(FX_BSTRC("FT"));
     if (!pType) {
       return FALSE;
     }
   }
@@ skipping 282 matching lines @@
     return FALSE;
   }
   FX_FILESIZE newxrefpos = GetDirectInteger(m_pTrailer, FX_BSTRC("Prev"));
   if (newxrefpos == xrefpos) {
     return FALSE;
   }
   xrefpos = newxrefpos;
   while (xrefpos) {
     CrossRefList.InsertAt(0, xrefpos);
     LoadCrossRefV4(xrefpos, 0, TRUE, FALSE);
-    CPDF_Dictionary* pDict = LoadTrailerV4();
-    if (pDict == NULL) {
+    nonstd::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>> pDict(
+        LoadTrailerV4());
+    if (!pDict)
       return FALSE;
-    }
-    if (!CheckDirectType(pDict, FX_BSTRC("Prev"), PDFOBJ_NUMBER)) {
-      pDict->Release();
+
+    if (!CheckDirectType(pDict.get(), FX_BSTRC("Prev"), PDFOBJ_NUMBER))
       return FALSE;
-    }
-    newxrefpos = GetDirectInteger(pDict, FX_BSTRC("Prev"));
-    if (newxrefpos == xrefpos) {
-      pDict->Release();
+
+    newxrefpos = GetDirectInteger(pDict.get(), FX_BSTRC("Prev"));
+    if (newxrefpos == xrefpos)
       return FALSE;
-    }
+
     xrefpos = newxrefpos;
     XRefStreamList.InsertAt(0, pDict->GetInteger(FX_BSTRC("XRefStm")));
-    m_Trailers.Add(pDict);
+    m_Trailers.Add(pDict.release());
   }
-  for (int32_t i = 0; i < CrossRefList.GetSize(); i++)
+  for (int32_t i = 0; i < CrossRefList.GetSize(); i++) {
     if (!LoadCrossRefV4(CrossRefList[i], XRefStreamList[i], FALSE, i == 0)) {

[Tom Sepez, 2015/10/09 16:47:31]

nit: no braces here.  Yes braces above.

[Lei Zhang, 2015/10/19 08:05:51]

Done.

       return FALSE;
     }
+  }
   return TRUE;
 }
 FX_BOOL CPDF_Parser::LoadLinearizedAllCrossRefV4(FX_FILESIZE xrefpos,
                                                  FX_DWORD dwObjCount) {
   if (!LoadLinearizedCrossRefV4(xrefpos, dwObjCount)) {
     return FALSE;
   }
   m_pTrailer = LoadTrailerV4();
   if (m_pTrailer == NULL) {
     return FALSE;
@@ skipping 795 matching lines @@
       m_SortedOffset.GetSize() - 1) {
     return FALSE;
   }
   FX_FILESIZE size = ((FX_FILESIZE*)pResult)[1] - pos;
   FX_FILESIZE SavedPos = m_Syntax.SavePos();
   m_Syntax.RestorePos(pos);
   bForm = m_Syntax.SearchMultiWord(FX_BSTRC("/Form\0stream"), TRUE, size) == 0;
   m_Syntax.RestorePos(SavedPos);
   return TRUE;
 }
+
 CPDF_Object* CPDF_Parser::ParseIndirectObject(CPDF_IndirectObjects* pObjList,
                                               FX_DWORD objnum,
                                               PARSE_CONTEXT* pContext) {
-  if (objnum >= (FX_DWORD)m_CrossRef.GetSize()) {
-    return NULL;
-  }
+  if (objnum >= (FX_DWORD)m_CrossRef.GetSize())
+    return nullptr;
+
   if (m_V5Type[objnum] == 1 || m_V5Type[objnum] == 255) {
     FX_FILESIZE pos = m_CrossRef[objnum];
-    if (pos <= 0) {
-      return NULL;
-    }
+    if (pos <= 0)
+      return nullptr;
     return ParseIndirectObjectAt(pObjList, pos, objnum, pContext);
   }
-  if (m_V5Type[objnum] == 2) {
-    CPDF_StreamAcc* pObjStream = GetObjectStream((FX_DWORD)m_CrossRef[objnum]);
-    if (pObjStream == NULL) {
-      return NULL;
+  if (m_V5Type[objnum] != 2)
+    return nullptr;
+
+  CPDF_StreamAcc* pObjStream = GetObjectStream((FX_DWORD)m_CrossRef[objnum]);
+  if (!pObjStream)
+    return nullptr;
+
+  nonstd::unique_ptr<IFX_FileStream, ReleaseDeleter<IFX_FileStream>> file(
+      FX_CreateMemoryStream((uint8_t*)pObjStream->GetData(),
+                            (size_t)pObjStream->GetSize(), FALSE));
+  CPDF_SyntaxParser syntax;
+  syntax.InitParser(file.get(), 0);
+  int32_t offset = GetStreamFirst(pObjStream);
+  for (int32_t i = GetStreamNCount(pObjStream); i > 0; --i) {
+    FX_DWORD thisnum = syntax.GetDirectNum();
+    FX_DWORD thisoff = syntax.GetDirectNum();
+    if (thisnum == objnum) {
+      syntax.RestorePos(offset + thisoff);
+      return syntax.GetObject(pObjList, 0, 0, pContext);
     }
-    int32_t n = pObjStream->GetDict()->GetInteger(FX_BSTRC("N"));
-    int32_t offset = pObjStream->GetDict()->GetInteger(FX_BSTRC("First"));
-    CPDF_SyntaxParser syntax;
-    CFX_SmartPointer<IFX_FileStream> file(FX_CreateMemoryStream(
-        (uint8_t*)pObjStream->GetData(), (size_t)pObjStream->GetSize(), FALSE));
-    syntax.InitParser(file.Get(), 0);
-    CPDF_Object* pRet = NULL;
-    while (n) {
-      FX_DWORD thisnum = syntax.GetDirectNum();
-      FX_DWORD thisoff = syntax.GetDirectNum();
-      if (thisnum == objnum) {
-        syntax.RestorePos(offset + thisoff);
-        pRet = syntax.GetObject(pObjList, 0, 0, pContext);
-        break;
-      }
-      n--;
-    }
-    return pRet;
   }
-  return NULL;
+  return nullptr;
 }
+
 CPDF_StreamAcc* CPDF_Parser::GetObjectStream(FX_DWORD objnum) {
   CPDF_StreamAcc* pStreamAcc = NULL;
   if (m_ObjectStreamMap.Lookup((void*)(uintptr_t)objnum, (void*&)pStreamAcc)) {
     return pStreamAcc;
   }
   const CPDF_Stream* pStream =
       m_pDocument ? (CPDF_Stream*)m_pDocument->GetIndirectObject(objnum) : NULL;
   if (pStream == NULL || pStream->GetType() != PDFOBJ_STREAM) {
     return NULL;
   }
@@ skipping 30 matching lines @@
 }
 void CPDF_Parser::GetIndirectBinary(FX_DWORD objnum,
                                     uint8_t*& pBuffer,
                                     FX_DWORD& size) {
   pBuffer = NULL;
   size = 0;
   if (objnum >= (FX_DWORD)m_CrossRef.GetSize()) {
     return;
   }
   if (m_V5Type[objnum] == 2) {
     CPDF_StreamAcc* pObjStream = GetObjectStream((FX_DWORD)m_CrossRef[objnum]);

[Tom Sepez, 2015/10/09 16:47:32]

nit: for another day, every time we seem to use m_CrossRef, there's a cast
involved.  Maybe its type should change?

[Lei Zhang, 2015/10/19 08:05:51]

Acknowledged.

-    if (pObjStream == NULL) {
+    if (!pObjStream)
       return;
-    }
-    int32_t n = pObjStream->GetDict()->GetInteger(FX_BSTRC("N"));
-    int32_t offset = pObjStream->GetDict()->GetInteger(FX_BSTRC("First"));
-    CPDF_SyntaxParser syntax;
+
+    int32_t offset = GetStreamFirst(pObjStream);
     const uint8_t* pData = pObjStream->GetData();
     FX_DWORD totalsize = pObjStream->GetSize();
-    CFX_SmartPointer<IFX_FileStream> file(
+    nonstd::unique_ptr<IFX_FileStream, ReleaseDeleter<IFX_FileStream>> file(
         FX_CreateMemoryStream((uint8_t*)pData, (size_t)totalsize, FALSE));
-    syntax.InitParser(file.Get(), 0);
-    while (n) {
+    CPDF_SyntaxParser syntax;
+    syntax.InitParser(file.get(), 0);
+    for (int i = GetStreamNCount(pObjStream); i > 0; --i) {
       FX_DWORD thisnum = syntax.GetDirectNum();
       FX_DWORD thisoff = syntax.GetDirectNum();
-      if (thisnum == objnum) {
-        if (n == 1) {
-          size = totalsize - (thisoff + offset);
-        } else {
-          syntax.GetDirectNum();  // Skip nextnum.
-          FX_DWORD nextoff = syntax.GetDirectNum();
-          size = nextoff - thisoff;
-        }
-        pBuffer = FX_Alloc(uint8_t, size);
-        FXSYS_memcpy(pBuffer, pData + thisoff + offset, size);
-        return;
+      if (thisnum != objnum)
+        continue;
+
+      if (i == 1) {
+        size = totalsize - (thisoff + offset);
+      } else {
+        syntax.GetDirectNum();  // Skip nextnum.
+        FX_DWORD nextoff = syntax.GetDirectNum();
+        size = nextoff - thisoff;
       }
-      n--;
+      pBuffer = FX_Alloc(uint8_t, size);
+      FXSYS_memcpy(pBuffer, pData + thisoff + offset, size);
+      return;
     }
     return;
   }
-  if (m_V5Type[objnum] == 1) {
-    FX_FILESIZE pos = m_CrossRef[objnum];
-    if (pos == 0) {
-      return;
-    }
-    FX_FILESIZE SavedPos = m_Syntax.SavePos();
-    m_Syntax.RestorePos(pos);
-    FX_BOOL bIsNumber;
-    CFX_ByteString word = m_Syntax.GetNextWord(bIsNumber);
-    if (!bIsNumber) {
-      m_Syntax.RestorePos(SavedPos);
-      return;
-    }
-    FX_DWORD parser_objnum = FXSYS_atoi(word);
-    if (parser_objnum && parser_objnum != objnum) {
-      m_Syntax.RestorePos(SavedPos);
-      return;
-    }
+
+  if (m_V5Type[objnum] != 1)
+    return;
+
+  FX_FILESIZE pos = m_CrossRef[objnum];
+  if (pos == 0) {
+    return;
+  }
+  FX_FILESIZE SavedPos = m_Syntax.SavePos();
+  m_Syntax.RestorePos(pos);
+  FX_BOOL bIsNumber;
+  CFX_ByteString word = m_Syntax.GetNextWord(bIsNumber);

[Tom Sepez, 2015/10/09 16:47:32]

nit: someday getnextword should take &bIsNumber rather than non-const ref.

[Lei Zhang, 2015/10/19 08:05:51]

Acknowledged.

+  if (!bIsNumber) {
+    m_Syntax.RestorePos(SavedPos);
+    return;
+  }
+  FX_DWORD parser_objnum = FXSYS_atoi(word);
+  if (parser_objnum && parser_objnum != objnum) {
+    m_Syntax.RestorePos(SavedPos);
+    return;
+  }
+  word = m_Syntax.GetNextWord(bIsNumber);
+  if (!bIsNumber) {
+    m_Syntax.RestorePos(SavedPos);
+    return;
+  }
+  if (m_Syntax.GetKeyword() != FX_BSTRC("obj")) {
+    m_Syntax.RestorePos(SavedPos);
+    return;
+  }
+  void* pResult =
+      FXSYS_bsearch(&pos, m_SortedOffset.GetData(), m_SortedOffset.GetSize(),
+                    sizeof(FX_FILESIZE), CompareFileSize);
+  if (pResult == NULL) {
+    m_Syntax.RestorePos(SavedPos);
+    return;
+  }
+  FX_FILESIZE nextoff = ((FX_FILESIZE*)pResult)[1];
+  FX_BOOL bNextOffValid = FALSE;
+  if (nextoff != pos) {
+    m_Syntax.RestorePos(nextoff);
     word = m_Syntax.GetNextWord(bIsNumber);
-    if (!bIsNumber) {
-      m_Syntax.RestorePos(SavedPos);
-      return;
-    }
-    if (m_Syntax.GetKeyword() != FX_BSTRC("obj")) {
-      m_Syntax.RestorePos(SavedPos);
-      return;
-    }
-    void* pResult =
-        FXSYS_bsearch(&pos, m_SortedOffset.GetData(), m_SortedOffset.GetSize(),
-                      sizeof(FX_FILESIZE), CompareFileSize);
-    if (pResult == NULL) {
-      m_Syntax.RestorePos(SavedPos);
-      return;
-    }
-    FX_FILESIZE nextoff = ((FX_FILESIZE*)pResult)[1];
-    FX_BOOL bNextOffValid = FALSE;
-    if (nextoff != pos) {
-      m_Syntax.RestorePos(nextoff);
+    if (word == FX_BSTRC("xref")) {
+      bNextOffValid = TRUE;
+    } else if (bIsNumber) {
       word = m_Syntax.GetNextWord(bIsNumber);
-      if (word == FX_BSTRC("xref")) {
+      if (bIsNumber && m_Syntax.GetKeyword() == FX_BSTRC("obj")) {
         bNextOffValid = TRUE;
-      } else if (bIsNumber) {
-        word = m_Syntax.GetNextWord(bIsNumber);
-        if (bIsNumber && m_Syntax.GetKeyword() == FX_BSTRC("obj")) {
-          bNextOffValid = TRUE;
-        }
       }
     }
-    if (!bNextOffValid) {
-      m_Syntax.RestorePos(pos);
-      while (1) {
-        if (m_Syntax.GetKeyword() == FX_BSTRC("endobj")) {
-          break;
-        }
-        if (m_Syntax.SavePos() == m_Syntax.m_FileLen) {
-          break;
-        }
+  }
+  if (!bNextOffValid) {
+    m_Syntax.RestorePos(pos);
+    while (1) {
+      if (m_Syntax.GetKeyword() == FX_BSTRC("endobj")) {
+        break;
       }
-      nextoff = m_Syntax.SavePos();
+      if (m_Syntax.SavePos() == m_Syntax.m_FileLen) {
+        break;
+      }
     }
-    size = (FX_DWORD)(nextoff - pos);
-    pBuffer = FX_Alloc(uint8_t, size);
-    m_Syntax.RestorePos(pos);
-    m_Syntax.ReadBlock(pBuffer, size);
-    m_Syntax.RestorePos(SavedPos);
+    nextoff = m_Syntax.SavePos();
   }
+  size = (FX_DWORD)(nextoff - pos);
+  pBuffer = FX_Alloc(uint8_t, size);
+  m_Syntax.RestorePos(pos);
+  m_Syntax.ReadBlock(pBuffer, size);
+  m_Syntax.RestorePos(SavedPos);
 }
+
 CPDF_Object* CPDF_Parser::ParseIndirectObjectAt(CPDF_IndirectObjects* pObjList,
                                                 FX_FILESIZE pos,
                                                 FX_DWORD objnum,
                                                 PARSE_CONTEXT* pContext) {
   FX_FILESIZE SavedPos = m_Syntax.SavePos();
   m_Syntax.RestorePos(pos);
   FX_BOOL bIsNumber;
   CFX_ByteString word = m_Syntax.GetNextWord(bIsNumber);
   if (!bIsNumber) {
     m_Syntax.RestorePos(SavedPos);
@@ skipping 62 matching lines @@
     return NULL;
   }
   CPDF_Object* pObj =
       m_Syntax.GetObjectByStrict(pObjList, objnum, gennum, pContext);
   if (pResultPos) {
     *pResultPos = m_Syntax.m_Pos;
   }
   m_Syntax.RestorePos(SavedPos);
   return pObj;
 }
+
 CPDF_Dictionary* CPDF_Parser::LoadTrailerV4() {
-  if (m_Syntax.GetKeyword() != FX_BSTRC("trailer")) {
-    return NULL;
-  }
-  CPDF_Object* pObj = m_Syntax.GetObject(m_pDocument, 0, 0, 0);
-  if (pObj == NULL || pObj->GetType() != PDFOBJ_DICTIONARY) {
-    if (pObj) {
-      pObj->Release();
-    }
-    return NULL;
-  }
-  return (CPDF_Dictionary*)pObj;
+  if (m_Syntax.GetKeyword() != FX_BSTRC("trailer"))
+    return nullptr;
+
+  nonstd::unique_ptr<CPDF_Object, ReleaseDeleter<CPDF_Object>> pObj(
+      m_Syntax.GetObject(m_pDocument, 0, 0, 0));
+  if (!pObj || pObj->GetType() != PDFOBJ_DICTIONARY)
+    return nullptr;
+  return static_cast<CPDF_Dictionary*>(pObj.release());
 }
+
 FX_DWORD CPDF_Parser::GetPermissions(FX_BOOL bCheckRevision) {
   if (m_pSecurityHandler == NULL) {
     return (FX_DWORD)-1;
   }
   FX_DWORD dwPermission = m_pSecurityHandler->GetPermissions();
   if (m_pEncryptDict &&
       m_pEncryptDict->GetString(FX_BSTRC("Filter")) == FX_BSTRC("Standard")) {
     dwPermission &= 0xFFFFFFFC;
     dwPermission |= 0xFFFFF0C0;
     if (bCheckRevision && m_pEncryptDict->GetInteger(FX_BSTRC("R")) == 2) {
@@ skipping 634 matching lines @@
     if (m_pCryptoHandler && bDecrypt) {
       m_pCryptoHandler->Decrypt(objnum, gennum, str);
     }
     return CPDF_String::Create(str, TRUE);
   }
   if (word == FX_BSTRC("[")) {
     if (bTypeOnly) {
       return (CPDF_Object*)PDFOBJ_ARRAY;
     }
     CPDF_Array* pArray = CPDF_Array::Create();
-    while (1) {
-      CPDF_Object* pObj = GetObject(pObjList, objnum, gennum);
-      if (pObj == NULL) {
-        return pArray;
-      }
+    while (CPDF_Object* pObj = GetObject(pObjList, objnum, gennum))
       pArray->Add(pObj);
-    }
+    return pArray;

[Tom Sepez, 2015/10/09 16:47:32]

nit: maybe blank line here.

[Lei Zhang, 2015/10/19 08:05:51]

Done.

   }
   if (word[0] == '/') {
     if (bTypeOnly) {
       return (CPDF_Object*)PDFOBJ_NAME;
     }
     return CPDF_Name::Create(
         PDF_NameDecode(CFX_ByteStringC(m_WordBuffer + 1, m_WordSize - 1)));
   }
   if (word == FX_BSTRC("<<")) {
-    if (bTypeOnly) {
+    if (bTypeOnly)
       return (CPDF_Object*)PDFOBJ_DICTIONARY;
-    }
-    if (pContext) {
+
+    if (pContext)
       pContext->m_DictStart = SavedPos;
-    }
-    CPDF_Dictionary* pDict = CPDF_Dictionary::Create();
+
     int32_t nKeys = 0;
     FX_FILESIZE dwSignValuePos = 0;
+    nonstd::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>> pDict(
+        CPDF_Dictionary::Create());
     while (1) {
       FX_BOOL bIsNumber;
       CFX_ByteString key = GetNextWord(bIsNumber);
-      if (key.IsEmpty()) {
-        if (pDict)
-          pDict->Release();
-        return NULL;
-      }
+      if (key.IsEmpty())
+        return nullptr;
+
       FX_FILESIZE SavedPos = m_Pos - key.GetLength();
-      if (key == FX_BSTRC(">>")) {
+      if (key == FX_BSTRC(">>"))
         break;
-      }
+
       if (key == FX_BSTRC("endobj")) {
         m_Pos = SavedPos;
         break;
       }
-      if (key[0] != '/') {
+      if (key[0] != '/')
         continue;
-      }
-      nKeys++;
+
+      ++nKeys;
       key = PDF_NameDecode(key);
-      if (key == FX_BSTRC("/Contents")) {
+      if (key == FX_BSTRC("/Contents"))
         dwSignValuePos = m_Pos;
-      }
+
       CPDF_Object* pObj = GetObject(pObjList, objnum, gennum);
-      if (pObj == NULL) {
+      if (!pObj)
         continue;
-      }
+
       if (key.GetLength() >= 1) {
         if (nKeys < 32) {
           pDict->SetAt(CFX_ByteStringC(key.c_str() + 1, key.GetLength() - 1),
                        pObj);
         } else {
           pDict->AddValue(CFX_ByteStringC(key.c_str() + 1, key.GetLength() - 1),
                           pObj);
         }
       }
     }
-    if (IsSignatureDict(pDict)) {
+
+    if (IsSignatureDict(pDict.get())) {
       FX_FILESIZE dwSavePos = m_Pos;
       m_Pos = dwSignValuePos;
       CPDF_Object* pObj = GetObject(pObjList, objnum, gennum, NULL, FALSE);
       pDict->SetAt(FX_BSTRC("Contents"), pObj);
       m_Pos = dwSavePos;
     }
     if (pContext) {
       pContext->m_DictEnd = m_Pos;
       if (pContext->m_Flags & PDFPARSE_NOSTREAM) {
-        return pDict;
+        return pDict.release();
       }
     }
     FX_FILESIZE SavedPos = m_Pos;
     FX_BOOL bIsNumber;
     CFX_ByteString nextword = GetNextWord(bIsNumber);
-    if (nextword == FX_BSTRC("stream")) {
-      CPDF_Stream* pStream = ReadStream(pDict, pContext, objnum, gennum);
-      if (pStream) {
-        return pStream;
-      }
-      if (pDict)
-        pDict->Release();
-      return NULL;
-    } else {
+    if (nextword != FX_BSTRC("stream")) {
       m_Pos = SavedPos;
-      return pDict;
+      return pDict.release();
     }
+
+    return ReadStream(pDict.release(), pContext, objnum, gennum);
   }
   if (word == FX_BSTRC(">>")) {
     m_Pos = SavedPos;
-    return NULL;
+    return nullptr;
   }
-  if (bTypeOnly) {
+  if (bTypeOnly)
     return (CPDF_Object*)PDFOBJ_INVALID;
-  }
-  return NULL;
+
+  return nullptr;
 }
+
 CPDF_Object* CPDF_SyntaxParser::GetObjectByStrict(
     CPDF_IndirectObjects* pObjList,
     FX_DWORD objnum,
     FX_DWORD gennum,
     struct PARSE_CONTEXT* pContext) {
   CFX_AutoRestorer<int> restorer(&s_CurrentRecursionDepth);
   if (++s_CurrentRecursionDepth > kParserMaxRecursionDepth) {
     return NULL;
   }
   FX_FILESIZE SavedPos = m_Pos;
@@ skipping 54 matching lines @@
     CFX_ByteString str = ReadHexString();
     if (m_pCryptoHandler) {
       m_pCryptoHandler->Decrypt(objnum, gennum, str);
     }
     return CPDF_String::Create(str, TRUE);
   }
   if (word == FX_BSTRC("[")) {
     if (bTypeOnly) {
       return (CPDF_Object*)PDFOBJ_ARRAY;
     }
-    CPDF_Array* pArray = CPDF_Array::Create();
-    while (1) {
-      CPDF_Object* pObj = GetObject(pObjList, objnum, gennum);
-      if (pObj == NULL) {
-        if (m_WordBuffer[0] == ']') {
-          return pArray;
-        }
-        if (pArray) {
-          pArray->Release();
-        }
-        return NULL;
-      }
+    nonstd::unique_ptr<CPDF_Array, ReleaseDeleter<CPDF_Array>> pArray(
+        CPDF_Array::Create());
+    while (CPDF_Object* pObj = GetObject(pObjList, objnum, gennum))
       pArray->Add(pObj);
-    }
+    return (m_WordBuffer[0] == ']') ? pArray.release() : nullptr;

[Tom Sepez, 2015/10/09 16:47:32]

nit: overparenthesized.

[Lei Zhang, 2015/10/19 08:05:51]

Done.

   }
   if (word[0] == '/') {
     if (bTypeOnly) {
       return (CPDF_Object*)PDFOBJ_NAME;
     }
     return CPDF_Name::Create(
         PDF_NameDecode(CFX_ByteStringC(m_WordBuffer + 1, m_WordSize - 1)));
   }
   if (word == FX_BSTRC("<<")) {
     if (bTypeOnly) {
       return (CPDF_Object*)PDFOBJ_DICTIONARY;
     }
     if (pContext) {
       pContext->m_DictStart = SavedPos;
     }
-    CPDF_Dictionary* pDict = CPDF_Dictionary::Create();
+    nonstd::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>> pDict(
+        CPDF_Dictionary::Create());
     while (1) {
       FX_BOOL bIsNumber;
       FX_FILESIZE SavedPos = m_Pos;
       CFX_ByteString key = GetNextWord(bIsNumber);
-      if (key.IsEmpty()) {
-        if (pDict) {
-          pDict->Release();
-        }
-        return NULL;
-      }
-      if (key == FX_BSTRC(">>")) {
+      if (key.IsEmpty())
+        return nullptr;
+
+      if (key == FX_BSTRC(">>"))
         break;
-      }
+
       if (key == FX_BSTRC("endobj")) {
         m_Pos = SavedPos;
         break;
       }
-      if (key[0] != '/') {
+      if (key[0] != '/')
         continue;
-      }
+
       key = PDF_NameDecode(key);
       nonstd::unique_ptr<CPDF_Object, ReleaseDeleter<CPDF_Object>> obj(
           GetObject(pObjList, objnum, gennum));
       if (!obj) {
-        if (pDict) {
-          pDict->Release();
-        }
         uint8_t ch;
         while (1) {
-          if (!GetNextChar(ch)) {
+          if (!GetNextChar(ch))

[Tom Sepez, 2015/10/09 16:47:32]

nit: maybe combine with subsequent if.
nit: then invert the test, and use in condition of while, so that we wind up
with something along the lines of while (...) continue;

[Lei Zhang, 2015/10/19 08:05:51]

Done.

             break;
-          }
-          if (ch == 0x0A || ch == 0x0D) {
+          if (ch == 0x0A || ch == 0x0D)

[Tom Sepez, 2015/10/09 16:47:32]

nit: do we want '\n' and '\r'?  A fascintating philosophical question about
whether the host character set is ascii vs. the document character set.  Hmmm.

[Lei Zhang, 2015/10/19 08:05:51]

Meh. I'm just going to leave it as is.

             break;
-          }
         }
-        return NULL;
+        return nullptr;
       }
       if (key.GetLength() > 1) {
         pDict->AddValue(CFX_ByteStringC(key.c_str() + 1, key.GetLength() - 1),
                         obj.release());
       }
     }
     if (pContext) {
       pContext->m_DictEnd = m_Pos;
       if (pContext->m_Flags & PDFPARSE_NOSTREAM) {
-        return pDict;
+        return pDict.release();
       }
     }
     FX_FILESIZE SavedPos = m_Pos;
     FX_BOOL bIsNumber;
     CFX_ByteString nextword = GetNextWord(bIsNumber);
-    if (nextword == FX_BSTRC("stream")) {
-      CPDF_Stream* pStream = ReadStream(pDict, pContext, objnum, gennum);
-      if (pStream) {
-        return pStream;
-      }
-      if (pDict) {
-        pDict->Release();
-      }
-      return NULL;
-    } else {
+    if (nextword != FX_BSTRC("stream")) {
       m_Pos = SavedPos;
-      return pDict;
+      return pDict.release();
     }
+
+    return ReadStream(pDict.release(), pContext, objnum, gennum);
   }
   if (word == FX_BSTRC(">>")) {
     m_Pos = SavedPos;
-    return NULL;
+    return nullptr;
   }
-  if (bTypeOnly) {
+  if (bTypeOnly)
     return (CPDF_Object*)PDFOBJ_INVALID;
-  }
-  return NULL;
+
+  return nullptr;
 }
+
 unsigned int CPDF_SyntaxParser::ReadEOLMarkers(FX_FILESIZE pos) {
   unsigned char byte1 = 0;
   unsigned char byte2 = 0;
   GetCharAt(pos, byte1);
   GetCharAt(pos + 1, byte2);
   unsigned int markers = 0;
   if (byte1 == '\r' && byte2 == '\n') {
     markers = 2;
   } else if (byte1 == '\r' || byte1 == '\n') {
     markers = 1;
@@ skipping 244 matching lines @@
       offset = byte == tag_data[taglen - 1] ? taglen - 2 : taglen - 1;
       pos--;
     }
     if (pos < 0) {
       return FALSE;
     }
   }
   return FALSE;
 }
 
-struct _SearchTagRecord {
-  const uint8_t* m_pTag;
-  FX_DWORD m_Len;
-  FX_DWORD m_Offset;
-};
-
 int32_t CPDF_SyntaxParser::SearchMultiWord(const CFX_ByteStringC& tags,
                                            FX_BOOL bWholeWord,
                                            FX_FILESIZE limit) {
   int32_t ntags = 1;
   for (int i = 0; i < tags.GetLength(); ++i) {
     if (tags[i] == 0) {
       ++ntags;
     }
   }
 
-  std::vector<_SearchTagRecord> patterns(ntags);

[Tom Sepez, 2015/10/09 16:47:31]

Good, _CAPITAL is reserved anyways.  This irks me no end.

[Lei Zhang, 2015/10/19 08:05:51]

Ack. There's plenty more in other files.

+  std::vector<SearchTagRecord> patterns(ntags);
   FX_DWORD start = 0;
   FX_DWORD itag = 0;
   FX_DWORD max_len = 0;
   for (int i = 0; i <= tags.GetLength(); ++i) {
     if (tags[i] == 0) {
       FX_DWORD len = i - start;
       max_len = std::max(len, max_len);
       patterns[itag].m_pTag = tags.GetPtr() + start;
       patterns[itag].m_Len = len;
       patterns[itag].m_Offset = 0;
       start = i + 1;
       ++itag;
     }
   }
 
   const FX_FILESIZE pos_limit = m_Pos + limit;
   for (FX_FILESIZE pos = m_Pos; !limit || pos < pos_limit; ++pos) {
     uint8_t byte;
     if (!GetCharAt(pos, byte))
       break;
 
     for (int i = 0; i < ntags; ++i) {

[Tom Sepez, 2015/10/09 16:47:32]

nit: can we use an iterator here now that you got a std::vector?

[Lei Zhang, 2015/10/19 08:05:51]

Annoyingly we still have a "return i" below in the loop, so it's not going to be
that much of a win.

-      _SearchTagRecord& pat = patterns[i];
+      SearchTagRecord& pat = patterns[i];
       if (pat.m_pTag[pat.m_Offset] != byte) {
         pat.m_Offset = (pat.m_pTag[0] == byte) ? 1 : 0;
         continue;
       }
 
       ++pat.m_Offset;
       if (pat.m_Offset != pat.m_Len)
         continue;
 
       if (!bWholeWord ||
@@ skipping 998 matching lines @@
     return PDF_UNKNOW_LINEARIZED;
   }
   uint8_t buffer[1024];
   m_pFileRead->ReadBlock(buffer, 0, req_size);
   if (IsLinearizedFile(buffer, req_size)) {
     return PDF_IS_LINEARIZED;
   }
   return PDF_NOT_LINEARIZED;
 }
 FX_BOOL CPDF_DataAvail::IsLinearizedFile(uint8_t* pData, FX_DWORD dwLen) {
-  CFX_SmartPointer<IFX_FileStream> file(
+  nonstd::unique_ptr<IFX_FileStream, ReleaseDeleter<IFX_FileStream>> file(
       FX_CreateMemoryStream(pData, (size_t)dwLen, FALSE));
-  int32_t offset = GetHeaderOffset(file.Get());
+  int32_t offset = GetHeaderOffset(file.get());
   if (offset == -1) {
     m_docStatus = PDF_DATAAVAIL_ERROR;
     return FALSE;
   }
   m_dwHeaderOffset = offset;
-  m_syntaxParser.InitParser(file.Get(), offset);
+  m_syntaxParser.InitParser(file.get(), offset);
   m_syntaxParser.RestorePos(m_syntaxParser.m_HeaderOffset + 9);
   FX_BOOL bNumber = FALSE;
   CFX_ByteString wordObjNum = m_syntaxParser.GetNextWord(bNumber);
   if (!bNumber) {
     return FALSE;
   }
   FX_DWORD objnum = FXSYS_atoi(wordObjNum);
   if (m_pLinearized) {
     m_pLinearized->Release();
     m_pLinearized = NULL;
@@ skipping 20 matching lines @@
     return TRUE;
   }
   return FALSE;
 }
 FX_BOOL CPDF_DataAvail::CheckEnd(IFX_DownloadHints* pHints) {
   FX_DWORD req_pos = (FX_DWORD)(m_dwFileLen > 1024 ? m_dwFileLen - 1024 : 0);
   FX_DWORD dwSize = (FX_DWORD)(m_dwFileLen - req_pos);
   if (m_pFileAvail->IsDataAvail(req_pos, dwSize)) {
     uint8_t buffer[1024];
     m_pFileRead->ReadBlock(buffer, req_pos, dwSize);
-    CFX_SmartPointer<IFX_FileStream> file(
+    nonstd::unique_ptr<IFX_FileStream, ReleaseDeleter<IFX_FileStream>> file(
         FX_CreateMemoryStream(buffer, (size_t)dwSize, FALSE));
-    m_syntaxParser.InitParser(file.Get(), 0);
+    m_syntaxParser.InitParser(file.get(), 0);
     m_syntaxParser.RestorePos(dwSize - 1);
     if (m_syntaxParser.SearchWord(FX_BSTRC("startxref"), TRUE, FALSE, dwSize)) {
       FX_BOOL bNumber;
       m_syntaxParser.GetNextWord(bNumber);
       CFX_ByteString xrefpos_str = m_syntaxParser.GetNextWord(bNumber);
       if (!bNumber) {
         m_docStatus = PDF_DATAAVAIL_ERROR;
         return FALSE;
       }
       m_dwXRefOffset = (FX_FILESIZE)FXSYS_atoi64(xrefpos_str);
@@ skipping 15 matching lines @@
 int32_t CPDF_DataAvail::CheckCrossRefStream(IFX_DownloadHints* pHints,
                                             FX_FILESIZE& xref_offset) {
   xref_offset = 0;
   FX_DWORD req_size =
       (FX_DWORD)(m_Pos + 512 > m_dwFileLen ? m_dwFileLen - m_Pos : 512);
   if (m_pFileAvail->IsDataAvail(m_Pos, req_size)) {
     int32_t iSize = (int32_t)(m_Pos + req_size - m_dwCurrentXRefSteam);
     CFX_BinaryBuf buf(iSize);
     uint8_t* pBuf = buf.GetBuffer();
     m_pFileRead->ReadBlock(pBuf, m_dwCurrentXRefSteam, iSize);
-    CFX_SmartPointer<IFX_FileStream> file(
+    nonstd::unique_ptr<IFX_FileStream, ReleaseDeleter<IFX_FileStream>> file(
         FX_CreateMemoryStream(pBuf, (size_t)iSize, FALSE));
-    m_parser.m_Syntax.InitParser(file.Get(), 0);
+    m_parser.m_Syntax.InitParser(file.get(), 0);
     FX_BOOL bNumber = FALSE;
     CFX_ByteString objnum = m_parser.m_Syntax.GetNextWord(bNumber);
     if (!bNumber) {
       return -1;
     }
     FX_DWORD objNum = FXSYS_atoi(objnum);
     CPDF_Object* pObj = m_parser.ParseIndirectObjectAt(NULL, 0, objNum, NULL);
     if (!pObj) {
       m_Pos += m_parser.m_Syntax.SavePos();
       return 0;
@@ skipping 217 matching lines @@
     int32_t iSize = (int32_t)(m_Pos + iTrailerSize - m_dwTrailerOffset);
     CFX_BinaryBuf buf(iSize);
     uint8_t* pBuf = buf.GetBuffer();
     if (!pBuf) {
       m_docStatus = PDF_DATAAVAIL_ERROR;
       return FALSE;
     }
     if (!m_pFileRead->ReadBlock(pBuf, m_dwTrailerOffset, iSize)) {
       return FALSE;
     }
-    CFX_SmartPointer<IFX_FileStream> file(
+    nonstd::unique_ptr<IFX_FileStream, ReleaseDeleter<IFX_FileStream>> file(
         FX_CreateMemoryStream(pBuf, (size_t)iSize, FALSE));
-    m_syntaxParser.InitParser(file.Get(), 0);
-    CPDF_Object* pTrailer = m_syntaxParser.GetObject(nullptr, 0, 0);
+    m_syntaxParser.InitParser(file.get(), 0);
+    nonstd::unique_ptr<CPDF_Object, ReleaseDeleter<CPDF_Object>> pTrailer(
+        m_syntaxParser.GetObject(nullptr, 0, 0));
     if (!pTrailer) {
       m_Pos += m_syntaxParser.SavePos();
       pHints->AddSegment(m_Pos, iTrailerSize);
       return FALSE;
     }
-    if (pTrailer->GetType() != PDFOBJ_DICTIONARY) {
-      pTrailer->Release();
+    if (pTrailer->GetType() != PDFOBJ_DICTIONARY)
       return FALSE;
-    }
+
     CPDF_Dictionary* pTrailerDict = pTrailer->GetDict();
     CPDF_Object* pEncrypt = pTrailerDict->GetElement("Encrypt");
     if (pEncrypt && pEncrypt->GetType() == PDFOBJ_REFERENCE) {
       m_docStatus = PDF_DATAAVAIL_LOADALLFILE;
-      pTrailer->Release();
       return TRUE;
     }
-    FX_DWORD xrefpos = GetDirectInteger(pTrailer->GetDict(), FX_BSTRC("Prev"));
+
+    FX_DWORD xrefpos = GetDirectInteger(pTrailerDict, FX_BSTRC("Prev"));
     if (xrefpos) {
-      m_dwPrevXRefOffset =
-          GetDirectInteger(pTrailer->GetDict(), FX_BSTRC("XRefStm"));
-      pTrailer->Release();
+      m_dwPrevXRefOffset = GetDirectInteger(pTrailerDict, FX_BSTRC("XRefStm"));
       if (m_dwPrevXRefOffset) {
         m_docStatus = PDF_DATAAVAIL_LOADALLFILE;
       } else {
         m_dwPrevXRefOffset = xrefpos;
         if (m_dwPrevXRefOffset >= m_dwFileLen) {
           m_docStatus = PDF_DATAAVAIL_LOADALLFILE;
         } else {
           SetStartOffset(m_dwPrevXRefOffset);
           m_docStatus = PDF_DATAAVAIL_TRAILER_APPEND;
         }
       }
       return TRUE;
     }
     m_dwPrevXRefOffset = 0;
     m_docStatus = PDF_DATAAVAIL_TRAILER_APPEND;
-    pTrailer->Release();
     return TRUE;
   }
   pHints->AddSegment(m_Pos, iTrailerSize);
   return FALSE;
 }
 
 FX_BOOL CPDF_DataAvail::CheckPage(int32_t iPage, IFX_DownloadHints* pHints) {
   while (TRUE) {
     switch (m_docStatus) {
       case PDF_DATAAVAIL_PAGETREE:
@@ skipping 533 matching lines @@
   return FALSE;
 }
 CPDF_PageNode::~CPDF_PageNode() {
   int32_t iSize = m_childNode.GetSize();
   for (int32_t i = 0; i < iSize; ++i) {
     CPDF_PageNode* pNode = (CPDF_PageNode*)m_childNode[i];
     delete pNode;
   }
   m_childNode.RemoveAll();
 }