Name the states for fpdf_parser_parser.

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 "core/include/fpdfapi/fpdf_parser.h"
 
 #include <algorithm>
 #include <memory>
@@ skipping 18 matching lines @@
 // A limit on the maximum object number in the xref table. Theoretical limits
 // are higher, but this may be large enough in practice.
 const FX_DWORD kMaxObjectNumber = 1048576;
 
 struct SearchTagRecord {
   const char* m_pTag;
   FX_DWORD m_Len;
   FX_DWORD m_Offset;
 };
 
+enum class ParserState {

[Tom Sepez, 2016/02/18 19:27:55]

Can we make ParserState an enum nested privately in CPDF_Parser?

[dsinclair, 2016/02/18 19:47:04]

Done.

Is there a benefit over having it in the anonymous namespace?

[Tom Sepez, 2016/02/18 19:49:16]

Yes, it tells the reader that this state is associated with the parser class and
only the parser class.

[dsinclair, 2016/02/18 19:51:03]

Ah, right. Too spoiled by one class per file. Wasn't thinking about the
many-classes per file we have.

+  kDefault,
+  kComment,

[Tom Sepez, 2016/02/18 19:27:55]

Can we keep these in the same order as before so that the values of the states
don't get permuted?  e.g. kWhiteSpace as 1?

I'm worried that in the depths of the logic, they may do some bizzaro thing like
if state > N or some such.

[dsinclair, 2016/02/18 19:47:04]

I checked through the code, all uses of status are to assign the new state or in
the switch(status). 

Putting in the existing order will force us to put in a dummy 12 state, which we
don't need.

[Tom Sepez, 2016/02/18 19:49:16]

Acknowledged.

+  kWhitespace,
+

[Tom Sepez, 2016/02/18 19:27:55]

nit: not sure we need blank lines here.

[dsinclair, 2016/02/18 19:47:04]

Done. Was an attempt at grouping and readability.

+  kString,
+  kHexString,
+  kEscapedString,
+
+  kXref,
+  kObjNum,
+  kPostObjNum,
+  kGenNum,
+  kPostGenNum,
+
+  kTrailer,
+
+  kBeginObj,
+  kEndObj
+};
+
 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) {
     if (!pFile->ReadBlock(buf, offset, kBufSize))
       return -1;
 
     if (*(FX_DWORD*)buf == tag)
@@ skipping 526 matching lines @@
   return TRUE;
 }
 
 FX_BOOL CPDF_Parser::RebuildCrossRef() {
   m_ObjectInfo.clear();
   m_SortedOffset.clear();
   if (m_pTrailer) {
     m_pTrailer->Release();
     m_pTrailer = NULL;
   }
-  int32_t status = 0;
+
+  enum ParserState status = ParserState::kDefault;

[Tom Sepez, 2016/02/18 19:27:55]

nit: |enum| needed ? Also, can we rename this |state| since "status" means
something slightly different (to me) -- its a "state machine", not a "status
machine". :).

[dsinclair, 2016/02/18 19:47:04]

Done.

+
   int32_t inside_index = 0;
   FX_DWORD objnum = 0;
   FX_DWORD gennum = 0;
   int32_t depth = 0;
   const FX_DWORD kBufferSize = 4096;
   std::vector<uint8_t> buffer(kBufferSize);
   FX_FILESIZE pos = m_Syntax.m_HeaderOffset;
   FX_FILESIZE start_pos = 0;
   FX_FILESIZE start_pos1 = 0;
   FX_FILESIZE last_obj = -1;
   FX_FILESIZE last_xref = -1;
   FX_FILESIZE last_trailer = -1;
   while (pos < m_Syntax.m_FileLen) {
     const FX_FILESIZE saved_pos = pos;
     bool bOverFlow = false;
     FX_DWORD size = std::min((FX_DWORD)(m_Syntax.m_FileLen - pos), kBufferSize);
     if (!m_Syntax.m_pFileAccess->ReadBlock(buffer.data(), pos, size))
       break;
 
     for (FX_DWORD i = 0; i < size; i++) {
       uint8_t byte = buffer[i];
       switch (status) {
-        case 0:
+        case ParserState::kDefault:
           if (PDFCharIsWhitespace(byte))
-            status = 1;
+            status = ParserState::kWhitespace;
 
           if (std::isdigit(byte)) {
             --i;
-            status = 1;
+            status = ParserState::kWhitespace;
           }
 
           if (byte == '%') {
             inside_index = 0;
-            status = 9;
+            status = ParserState::kComment;
           }
 
           if (byte == '(') {
-            status = 10;
+            status = ParserState::kString;
             depth = 1;
           }
 
           if (byte == '<') {
             inside_index = 1;
-            status = 11;
+            status = ParserState::kHexString;
           }
 
           if (byte == '\\')
-            status = 13;
+            status = ParserState::kEscapedString;
 
           if (byte == 't') {
-            status = 7;
+            status = ParserState::kTrailer;
             inside_index = 1;
           }
           break;
-        case 1:
+
+        case ParserState::kWhitespace:
           if (PDFCharIsWhitespace(byte)) {
             break;
           } else if (std::isdigit(byte)) {
             start_pos = pos + i;
-            status = 2;
+            status = ParserState::kObjNum;
             objnum = FXSYS_toDecimalDigit(byte);
+
           } else if (byte == 't') {
-            status = 7;
+            status = ParserState::kTrailer;
             inside_index = 1;
+
           } else if (byte == 'x') {
-            status = 8;
+            status = ParserState::kXref;
             inside_index = 1;
+
           } else {
             --i;
-            status = 0;
+            status = ParserState::kDefault;
           }
           break;
-        case 2:
+
+        case ParserState::kObjNum:
           if (std::isdigit(byte)) {
             objnum = objnum * 10 + FXSYS_toDecimalDigit(byte);
             break;
           } else if (PDFCharIsWhitespace(byte)) {
-            status = 3;
+            status = ParserState::kPostObjNum;
           } else {
             --i;
-            status = 14;
+            status = ParserState::kEndObj;
             inside_index = 0;
           }
           break;
-        case 3:
+
+        case ParserState::kPostObjNum:
           if (std::isdigit(byte)) {
             start_pos1 = pos + i;
-            status = 4;
+            status = ParserState::kGenNum;
             gennum = FXSYS_toDecimalDigit(byte);
           } else if (PDFCharIsWhitespace(byte)) {
             break;
           } else if (byte == 't') {
-            status = 7;
+            status = ParserState::kTrailer;
             inside_index = 1;
           } else {
             --i;
-            status = 0;
+            status = ParserState::kDefault;
           }
           break;
-        case 4:
+
+        case ParserState::kGenNum:
           if (std::isdigit(byte)) {
             gennum = gennum * 10 + FXSYS_toDecimalDigit(byte);
             break;
           } else if (PDFCharIsWhitespace(byte)) {
-            status = 5;
+            status = ParserState::kPostGenNum;
           } else {
             --i;
-            status = 0;
+            status = ParserState::kDefault;
           }
           break;
-        case 5:
+
+        case ParserState::kPostGenNum:
           if (byte == 'o') {
-            status = 6;
+            status = ParserState::kBeginObj;
             inside_index = 1;
           } else if (PDFCharIsWhitespace(byte)) {
             break;
           } else if (std::isdigit(byte)) {
             objnum = gennum;
             gennum = FXSYS_toDecimalDigit(byte);
             start_pos = start_pos1;
             start_pos1 = pos + i;
-            status = 4;
+            status = ParserState::kGenNum;
           } else if (byte == 't') {
-            status = 7;
+            status = ParserState::kTrailer;
             inside_index = 1;
           } else {
             --i;
-            status = 0;
+            status = ParserState::kDefault;
           }
           break;
-        case 6:
+
+        case ParserState::kBeginObj:
           switch (inside_index) {
             case 1:
               if (byte != 'b') {
                 --i;
-                status = 0;
+                status = ParserState::kDefault;
               } else {
                 inside_index++;
               }
               break;
             case 2:
               if (byte != 'j') {
                 --i;
-                status = 0;
+                status = ParserState::kDefault;
               } else {
                 inside_index++;
               }
               break;
             case 3:
               if (PDFCharIsWhitespace(byte) || PDFCharIsDelimiter(byte)) {
                 if (objnum > 0x1000000) {
-                  status = 0;
+                  status = ParserState::kDefault;
                   break;
                 }
                 FX_FILESIZE obj_pos = start_pos - m_Syntax.m_HeaderOffset;
                 m_SortedOffset.insert(obj_pos);
                 last_obj = start_pos;
                 FX_FILESIZE obj_end = 0;
                 CPDF_Object* pObject = ParseIndirectObjectAtByStrict(
                     m_pDocument, obj_pos, objnum, &obj_end);
                 if (CPDF_Stream* pStream = ToStream(pObject)) {
                   if (CPDF_Dictionary* pDict = pStream->GetDict()) {
@@ skipping 38 matching lines @@
                 } else {
                   m_ObjectInfo[objnum].pos = obj_pos;
                   m_ObjectInfo[objnum].type = 1;
                   m_ObjectInfo[objnum].gennum = gennum;
                 }
                 if (pObject) {
                   pObject->Release();
                 }
               }
               --i;
-              status = 0;
+              status = ParserState::kDefault;
               break;
           }
           break;
-        case 7:
+
+        case ParserState::kTrailer:
           if (inside_index == 7) {
             if (PDFCharIsWhitespace(byte) || PDFCharIsDelimiter(byte)) {
               last_trailer = pos + i - 7;
               m_Syntax.RestorePos(pos + i - m_Syntax.m_HeaderOffset);
               CPDF_Object* pObj = m_Syntax.GetObject(m_pDocument, 0, 0, true);
               if (pObj) {
                 if (!pObj->IsDictionary() && !pObj->AsStream()) {
                   pObj->Release();
                 } else {
                   CPDF_Stream* pStream = pObj->AsStream();
@@ skipping 42 matching lines @@
                       }
                       m_Syntax.RestorePos(dwSavePos);
                     }
                   } else {
                     pObj->Release();
                   }
                 }
               }
             }
             --i;
-            status = 0;
+            status = ParserState::kDefault;
           } else if (byte == "trailer"[inside_index]) {
             inside_index++;
           } else {
             --i;
-            status = 0;
+            status = ParserState::kDefault;
           }
           break;
-        case 8:
+
+        case ParserState::kXref:
           if (inside_index == 4) {
             last_xref = pos + i - 4;
-            status = 1;
+            status = ParserState::kWhitespace;
           } else if (byte == "xref"[inside_index]) {
             inside_index++;
           } else {
             --i;
-            status = 0;
+            status = ParserState::kDefault;
           }
           break;
-        case 9:
+
+        case ParserState::kComment:
           if (byte == '\r' || byte == '\n') {
-            status = 0;
+            status = ParserState::kDefault;
           }
           break;
-        case 10:
+
+        case ParserState::kString:
           if (byte == ')') {
             if (depth > 0) {
               depth--;
             }
           } else if (byte == '(') {
             depth++;
           }
           if (!depth) {
-            status = 0;
+            status = ParserState::kDefault;
           }
           break;
-        case 11:
+
+        case ParserState::kHexString:
           if (byte == '>' || (byte == '<' && inside_index == 1))
-            status = 0;
+            status = ParserState::kDefault;
           inside_index = 0;
           break;
-        case 13:

[Tom Sepez, 2016/02/18 19:27:55]

Heh.  No 12.

[dsinclair, 2016/02/18 19:47:04]

Acknowledged.

+
+        case ParserState::kEscapedString:
           if (PDFCharIsDelimiter(byte) || PDFCharIsWhitespace(byte)) {
             --i;
-            status = 0;
+            status = ParserState::kDefault;
           }
           break;
-        case 14:
+
+        case ParserState::kEndObj:
           if (PDFCharIsWhitespace(byte)) {
-            status = 0;
+            status = ParserState::kDefault;
           } else if (byte == '%' || byte == '(' || byte == '<' ||
                      byte == '\\') {
-            status = 0;
+            status = ParserState::kDefault;
             --i;
           } else if (inside_index == 6) {
-            status = 0;
+            status = ParserState::kDefault;
             --i;
           } else if (byte == "endobj"[inside_index]) {
             inside_index++;
           }
           break;
       }
       if (bOverFlow) {
         size = 0;
         break;
       }
@@ skipping 3793 matching lines @@
   if (!m_pLinearizedDict)
     return -1;
   CPDF_Array* pRange = m_pLinearizedDict->GetArrayBy("H");
   if (!pRange)
     return -1;
   CPDF_Object* pStreamLen = pRange->GetElementValue(1);
   if (!pStreamLen)
     return -1;
   return pStreamLen->GetInteger();
 }