War on #defines - part 2

fpdfsdk/src/fpdfview.cpp

Index: fpdfsdk/src/fpdfview.cpp
diff --git a/fpdfsdk/src/fpdfview.cpp b/fpdfsdk/src/fpdfview.cpp
index ccd127c5594ff1babb16a56c14f49dcef0f940bf..6e9ee05ea08be5ebc202068cd1f7f02fb2c8f820 100644
--- a/fpdfsdk/src/fpdfview.cpp
+++ b/fpdfsdk/src/fpdfview.cpp
@@ -274,19 +274,23 @@ int GetLastError() {
 }
 #endif  // _WIN32
 
-void ProcessParseError(FX_DWORD err_code) {
+void ProcessParseError(CPDF_Parser::Error err) {
+  FX_DWORD err_code;
   // Translate FPDFAPI error code to FPDFVIEW error code
-  switch (err_code) {
-    case PDFPARSE_ERROR_FILE:
+  switch (err) {
+    case CPDF_Parser::SUCCESS:
+      err_code = FPDF_ERR_SUCCESS;

[Lei Zhang, 2016/01/26 22:13:44]

This is NOTREACHED(), BTW.

[Tom Sepez, 2016/01/26 22:19:35]

It exists just to satisfy the all enums handled by switch requirement.

[brucedawson, 2016/02/03 19:49:08]

Was clang giving a warning without this case? Arguably a default case would be
better because now VC++'s /analyze warns that err_code is potentially not
initialized. Ah, the joys of trying to write clean code while also keeping
multiple compilers warning clean.

Could also avoid the VC++ warning by initializing err_code to FPDF_ERR_SUCCESS.
Or just ignore.

+      break;
+    case CPDF_Parser::FILE_ERROR:
       err_code = FPDF_ERR_FILE;
       break;
-    case PDFPARSE_ERROR_FORMAT:
+    case CPDF_Parser::FORMAT_ERROR:
       err_code = FPDF_ERR_FORMAT;
       break;
-    case PDFPARSE_ERROR_PASSWORD:
+    case CPDF_Parser::PASSWORD_ERROR:
       err_code = FPDF_ERR_PASSWORD;
       break;
-    case PDFPARSE_ERROR_HANDLER:
+    case CPDF_Parser::HANDLER_ERROR:
       err_code = FPDF_ERR_SECURITY;
       break;
   }
@@ -310,10 +314,10 @@ DLLEXPORT FPDF_DOCUMENT STDCALL FPDF_LoadDocument(FPDF_STRING file_path,
   CPDF_Parser* pParser = new CPDF_Parser;
   pParser->SetPassword(password);
 
-  FX_DWORD err_code = pParser->StartParse(pFileAccess);
-  if (err_code) {
+  CPDF_Parser::Error error = pParser->StartParse(pFileAccess);
+  if (error != CPDF_Parser::SUCCESS) {
     delete pParser;
-    ProcessParseError(err_code);
+    ProcessParseError(error);
     return NULL;
   }
 #ifdef PDF_ENABLE_XFA
@@ -399,15 +403,15 @@ DLLEXPORT FPDF_DOCUMENT STDCALL FPDF_LoadMemDocument(const void* data_buf,
   CPDF_Parser* pParser = new CPDF_Parser;
   pParser->SetPassword(password);
   CMemFile* pMemFile = new CMemFile((uint8_t*)data_buf, size);
-  FX_DWORD err_code = pParser->StartParse(pMemFile);
-  if (err_code) {
+  CPDF_Parser::Error error = pParser->StartParse(pMemFile);
+  if (error != CPDF_Parser::SUCCESS) {
     delete pParser;
-    ProcessParseError(err_code);
+    ProcessParseError(error);
     return NULL;
   }
   CPDF_Document* pDoc = NULL;
   pDoc = pParser ? pParser->GetDocument() : NULL;
-  CheckUnSupportError(pDoc, err_code);
+  CheckUnSupportError(pDoc, error);

[Lei Zhang, 2016/01/26 22:13:44]

BTW, all the CheckUnSupportError() errors pass |error| = SUCCESS.

[Tom Sepez, 2016/01/26 22:19:35]

Thanks.  Next time.

   return FPDFDocumentFromCPDFDocument(pParser->GetDocument());
 }
 
@@ -417,15 +421,15 @@ FPDF_LoadCustomDocument(FPDF_FILEACCESS* pFileAccess,
   CPDF_Parser* pParser = new CPDF_Parser;
   pParser->SetPassword(password);
   CPDF_CustomAccess* pFile = new CPDF_CustomAccess(pFileAccess);
-  FX_DWORD err_code = pParser->StartParse(pFile);
-  if (err_code) {
+  CPDF_Parser::Error error = pParser->StartParse(pFile);
+  if (error != CPDF_Parser::SUCCESS) {
     delete pParser;
-    ProcessParseError(err_code);
+    ProcessParseError(error);
     return NULL;
   }
   CPDF_Document* pDoc = NULL;
   pDoc = pParser ? pParser->GetDocument() : NULL;
-  CheckUnSupportError(pDoc, err_code);
+  CheckUnSupportError(pDoc, error);
   return FPDFDocumentFromCPDFDocument(pParser->GetDocument());
 }