Get rid of gotos in CPDF_SyntaxParser and FlateUncompress().

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"
 
 FX_BOOL IsSignatureDict(const CPDF_Dictionary* pDict) {
   CPDF_Object* pType = pDict->GetElementValue(FX_BSTRC("Type"));
   if (!pType) {
     pType = pDict->GetElementValue(FX_BSTRC("FT"));
@@ skipping 456 matching lines @@
           }
         }
         m_V5Type.SetAtGrow(objnum, 1);
       }
     }
   }
   FX_Free(pBuf);
   m_Syntax.RestorePos(SavedPos + count * recordsize);
   return TRUE;
 }
+
 FX_BOOL CPDF_Parser::LoadCrossRefV4(FX_FILESIZE pos,
                                     FX_FILESIZE streampos,
                                     FX_BOOL bSkip,
                                     FX_BOOL bFirst) {
   m_Syntax.RestorePos(pos);
   if (m_Syntax.GetKeyword() != FX_BSTRC("xref")) {
     return FALSE;
   }
   void* pResult =
       FXSYS_bsearch(&pos, m_SortedOffset.GetData(), m_SortedOffset.GetSize(),
@@ skipping 88 matching lines @@
           }
           if (bFirstBlock) {
             bFirstBlock = FALSE;
           }
         }
       }
       FX_Free(pBuf);
     }
     m_Syntax.RestorePos(SavedPos + count * recordsize);
   }
-  if (streampos)
-    if (!LoadCrossRefV5(streampos, streampos, FALSE)) {
-      return FALSE;
-    }
+  if (streampos && !LoadCrossRefV5(streampos, streampos, FALSE))

[Tom Sepez, 2015/09/23 21:48:41]

nit: maybe return !streampos || LoadCrossRefV5(..);
or not if you think its confusing.

[Lei Zhang, 2015/09/25 09:33:48]

Done.

+    return FALSE;
   return TRUE;
 }
+
 FX_BOOL CPDF_Parser::LoadAllCrossRefV5(FX_FILESIZE xrefpos) {
   if (!LoadCrossRefV5(xrefpos, xrefpos, TRUE)) {
     return FALSE;
   }
   while (xrefpos)
     if (!LoadCrossRefV5(xrefpos, xrefpos, FALSE)) {
       return FALSE;
     }
   m_ObjectStreamMap.InitHashTable(101, FALSE);
   m_bXRefStream = TRUE;
@@ skipping 2082 matching lines @@
     } else {
       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, i;
-  for (i = 0; i < tags.GetLength(); i++)
+  int32_t ntags = 1;
+  for (int i = 0; i < tags.GetLength(); i++) {
     if (tags[i] == 0) {
       ntags++;
     }
-  _SearchTagRecord* pPatterns = FX_Alloc(_SearchTagRecord, ntags);
-  FX_DWORD start = 0, itag = 0, max_len = 0;
-  for (i = 0; i <= tags.GetLength(); i++) {
+  }
+  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;
       if (len > max_len) {
         max_len = len;
       }
-      pPatterns[itag].m_pTag = tags.GetPtr() + start;
-      pPatterns[itag].m_Len = len;
-      pPatterns[itag].m_Offset = 0;
+      patterns[itag].m_pTag = tags.GetPtr() + start;
+      patterns[itag].m_Len = len;
+      patterns[itag].m_Offset = 0;
       start = i + 1;
       itag++;
     }
   }
-  FX_FILESIZE pos = m_Pos;
   uint8_t byte;
-  GetCharAt(pos++, byte);
-  int32_t found = -1;
-  while (1) {
-    for (i = 0; i < ntags; i++) {
-      if (pPatterns[i].m_pTag[pPatterns[i].m_Offset] == byte) {
-        pPatterns[i].m_Offset++;
-        if (pPatterns[i].m_Offset == pPatterns[i].m_Len) {
+  GetCharAt(m_Pos, byte);
+  for (FX_FILESIZE pos = m_Pos + 1;; ++pos) {
+    for (int i = 0; i < ntags; i++) {
+      if (patterns[i].m_pTag[patterns[i].m_Offset] == byte) {
+        patterns[i].m_Offset++;
+        if (patterns[i].m_Offset == patterns[i].m_Len) {
           if (!bWholeWord ||
-              IsWholeWord(pos - pPatterns[i].m_Len, limit, pPatterns[i].m_pTag,
-                          pPatterns[i].m_Len)) {
-            found = i;
-            goto end;
+              IsWholeWord(pos - patterns[i].m_Len, limit, patterns[i].m_pTag,
+                          patterns[i].m_Len)) {
+            return i;
           } else {
-            if (pPatterns[i].m_pTag[0] == byte) {
-              pPatterns[i].m_Offset = 1;
+            if (patterns[i].m_pTag[0] == byte) {
+              patterns[i].m_Offset = 1;
             } else {
-              pPatterns[i].m_Offset = 0;
+              patterns[i].m_Offset = 0;
             }
           }
         }
       } else {
-        if (pPatterns[i].m_pTag[0] == byte) {
-          pPatterns[i].m_Offset = 1;
+        if (patterns[i].m_pTag[0] == byte) {
+          patterns[i].m_Offset = 1;
         } else {
-          pPatterns[i].m_Offset = 0;
+          patterns[i].m_Offset = 0;
         }
       }
     }
-    if (limit && pos >= m_Pos + limit) {
-      goto end;
+
+    if ((limit && pos >= m_Pos + limit) || !GetCharAt(pos, byte)) {

[Tom Sepez, 2015/09/23 21:48:41]

I would like to have the first half of this || used in some form as the
condition in the for loop at 2739. Also, the getcharat can then be first in the
loop rather than last (unify with line 2738, and note that line's return value
isn't checked) so that we work on a new byte each time rather than the residue
of the last iteration. Let me know if there's a gotcha that prevents such a
form.

[Lei Zhang, 2015/09/25 09:33:48]

Done.

+      return -1;
     }
-    if (!GetCharAt(pos, byte)) {
-      goto end;
-    }
-    pos++;
   }
-end:
-  FX_Free(pPatterns);
-  return found;
+  return -1;

[Tom Sepez, 2015/09/23 21:48:41]

notreached? Only way out of your loop by return? want to break instead at 2766?

[Lei Zhang, 2015/09/25 09:33:48]

Acknowledged.

 }
+
 FX_FILESIZE CPDF_SyntaxParser::FindTag(const CFX_ByteStringC& tag,
                                        FX_FILESIZE limit) {
   int32_t taglen = tag.GetLength();
   int32_t match = 0;
   limit += m_Pos;
   FX_FILESIZE startpos = m_Pos;
   while (1) {
     uint8_t ch;
     if (!GetNextChar(ch)) {
       return -1;
@@ skipping 24 matching lines @@
       break;
     }
   }
 }
 
 class CPDF_DataAvail final : public IPDF_DataAvail {
  public:
   CPDF_DataAvail(IFX_FileAvail* pFileAvail, IFX_FileRead* pFileRead);
   ~CPDF_DataAvail() override;
 
-  virtual FX_BOOL IsDocAvail(IFX_DownloadHints* pHints) override;
+  FX_BOOL IsDocAvail(IFX_DownloadHints* pHints) override;
 
-  virtual void SetDocument(CPDF_Document* pDoc) override;
+  void SetDocument(CPDF_Document* pDoc) override;
 
-  virtual FX_BOOL IsPageAvail(int iPage, IFX_DownloadHints* pHints) override;
+  FX_BOOL IsPageAvail(int iPage, IFX_DownloadHints* pHints) override;
 
-  virtual int32_t IsFormAvail(IFX_DownloadHints* pHints) override;
+  int32_t IsFormAvail(IFX_DownloadHints* pHints) override;
 
-  virtual int32_t IsLinearizedPDF() override;
+  int32_t IsLinearizedPDF() override;
 
-  virtual FX_BOOL IsLinearized() override { return m_bLinearized; }
+  FX_BOOL IsLinearized() override { return m_bLinearized; }
 
-  virtual void GetLinearizedMainXRefInfo(FX_FILESIZE* pPos,
-                                         FX_DWORD* pSize) override;
+  void GetLinearizedMainXRefInfo(FX_FILESIZE* pPos, FX_DWORD* pSize) override;
 
  protected:
   static const int kMaxDataAvailRecursionDepth = 64;
   static int s_CurrentDataAvailRecursionDepth;
 
   FX_DWORD GetObjectSize(FX_DWORD objnum, FX_FILESIZE& offset);
   FX_BOOL IsObjectsAvail(CFX_PtrArray& obj_array,
                          FX_BOOL bParsePage,
                          IFX_DownloadHints* pHints,
                          CFX_PtrArray& ret_array);
@@ skipping 1842 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();
 }