Treat warnings as error and fix most level 4 warnings in the breakpad windows client projects.

Some of the lint errors in the files touched by this change were also fixed.

BUG=533
Review URL: https://breakpad.appspot.com/601002

git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1189 4c0a9323-5329-0410-9bdc-e9ce6186880e

8 files changed, 74 insertions, 67 deletions

diff --git a/src/client/windows/build/common.gypi b/src/client/windows/build/common.gypi
index dfd29bd0..60a24582 100644
--- a/src/client/windows/build/common.gypi
+++ b/src/client/windows/build/common.gypi
@@ -507,7 +507,7 @@
             'msvs_disabled_warnings': [4800],
             'msvs_settings': {
               'VCCLCompilerTool': {
-                'WarnAsError': 'false',
+                'WarnAsError': 'true',
                 'Detect64BitPortabilityProblems': 'false',
               },
             },
@@ -1174,7 +1174,7 @@
           '$(VSInstallDir)/VC/atlmfc/include',
         ],
         'msvs_cygwin_dirs': ['<(DEPTH)/third_party/cygwin'],
-        'msvs_disabled_warnings': [4396, 4503, 4819],
+        'msvs_disabled_warnings': [4100, 4127, 4396, 4503, 4512, 4819, 4995],
         'msvs_settings': {
           'VCCLCompilerTool': {
             'MinimalRebuild': 'false',
@@ -1182,7 +1182,7 @@
             'BufferSecurityCheck': 'true',
             'EnableFunctionLevelLinking': 'true',
             'RuntimeTypeInfo': 'false',
-            'WarningLevel': '3',
+            'WarningLevel': '4',
             'WarnAsError': 'true',
             'DebugInformationFormat': '3',
             'conditions': [
diff --git a/src/client/windows/handler/exception_handler.cc b/src/client/windows/handler/exception_handler.cc
index 14c2cf86..3a2f824c 100644
--- a/src/client/windows/handler/exception_handler.cc
+++ b/src/client/windows/handler/exception_handler.cc
@@ -96,7 +96,7 @@ ExceptionHandler::ExceptionHandler(const wstring& dump_path,
              pipe_handle,
              NULL,  // crash_generation_client
              custom_info);
-}  
+}
 
 ExceptionHandler::ExceptionHandler(
     const wstring& dump_path,
@@ -104,19 +104,19 @@ ExceptionHandler::ExceptionHandler(
     MinidumpCallback callback,
     void* callback_context,
     int handler_types,
-    MINIDUMP_TYPE dump_type,
-    CrashGenerationClient* crash_generation_client,
-    const CustomClientInfo* custom_info) {
+    CrashGenerationClient* crash_generation_client) {
+  // The dump_type, pipe_name and custom_info that are passed in to Initialize()
+  // are not used.  The ones set in crash_generation_client are used instead.
   Initialize(dump_path,
              filter,
              callback,
              callback_context,
              handler_types,
-             MiniDumpNormal,
-             NULL,  // pipe_name
-             NULL,  // pipe_handle
+             MiniDumpNormal,           // dump_type - not used
+             NULL,                     // pipe_name - not used
+             NULL,                     // pipe_handle
              crash_generation_client,
-             custom_info);
+             NULL);                    // custom_info - not used
 }
 
 ExceptionHandler::ExceptionHandler(const wstring &dump_path,
@@ -875,7 +875,7 @@ BOOL CALLBACK ExceptionHandler::MinidumpWriteDumpCallback(
     callback_context->iter++;
     return TRUE;
   }
-    
+
     // Include all modules.
   case IncludeModuleCallback:
   case ModuleCallback:
diff --git a/src/client/windows/handler/exception_handler.h b/src/client/windows/handler/exception_handler.h
index 2c8ff7d9..539c6662 100644
--- a/src/client/windows/handler/exception_handler.h
+++ b/src/client/windows/handler/exception_handler.h
@@ -61,9 +61,9 @@
 #include <DbgHelp.h>
 #include <rpc.h>
 
-#pragma warning( push )
+#pragma warning(push)
 // Disable exception handler warnings.
-#pragma warning( disable : 4530 )
+#pragma warning(disable:4530)
 
 #include <list>
 #include <string>
@@ -212,9 +212,7 @@ class ExceptionHandler {
                    MinidumpCallback callback,
                    void* callback_context,
                    int handler_types,
-                   MINIDUMP_TYPE dump_type,
-                   CrashGenerationClient* crash_generation_client,
-                   const CustomClientInfo* custom_info);
+                   CrashGenerationClient* crash_generation_client);
 
   ~ExceptionHandler();
 
@@ -497,7 +495,7 @@ class ExceptionHandler {
   static CRITICAL_SECTION handler_stack_critical_section_;
 
   // The number of instances of this class.
-  volatile static LONG instance_count_;
+  static volatile LONG instance_count_;
 
   // disallow copy ctor and operator=
   explicit ExceptionHandler(const ExceptionHandler &);
@@ -506,6 +504,6 @@ class ExceptionHandler {
 
 }  // namespace google_breakpad
 
-#pragma warning( pop )
+#pragma warning(pop)
 
 #endif  // CLIENT_WINDOWS_HANDLER_EXCEPTION_HANDLER_H__
diff --git a/src/client/windows/tests/crash_generation_app/crash_generation_app.cc b/src/client/windows/tests/crash_generation_app/crash_generation_app.cc
index e36c1d1e..12449ce9 100644
--- a/src/client/windows/tests/crash_generation_app/crash_generation_app.cc
+++ b/src/client/windows/tests/crash_generation_app/crash_generation_app.cc
@@ -345,9 +345,9 @@ void CleanUp() {
 
 // Processes messages for the main window.
 //
-// WM_COMMAND	- process the application menu.
-// WM_PAINT	- Paint the main window.
-// WM_DESTROY	- post a quit message and return.
+// WM_COMMAND - process the application menu.
+// WM_PAINT   - Paint the main window.
+// WM_DESTROY - post a quit message and return.
 LRESULT CALLBACK WndProc(HWND wnd,
                          UINT message,
                          WPARAM w_param,
@@ -359,7 +359,7 @@ LRESULT CALLBACK WndProc(HWND wnd,
 
 #pragma warning(push)
 #pragma warning(disable:4312)
-  // Disable warning	C4312: 'type cast' : conversion from 'LONG' to
+  // Disable warning C4312: 'type cast' : conversion from 'LONG' to
   // 'HINSTANCE' of greater size.
   // The value returned by GetwindowLong in the case below returns unsigned.
   HINSTANCE instance = (HINSTANCE)GetWindowLong(wnd, GWL_HINSTANCE);
@@ -415,16 +415,16 @@ LRESULT CALLBACK WndProc(HWND wnd,
                                             instance,
                                             NULL);
       break;
-    case WM_SIZE: 
-      // Make the edit control the size of the window's client area. 
-      MoveWindow(client_status_edit_box, 
+    case WM_SIZE:
+      // Make the edit control the size of the window's client area.
+      MoveWindow(client_status_edit_box,
                  0,
                  0,
                  LOWORD(l_param),        // width of client area.
                  HIWORD(l_param),        // height of client area.
                  TRUE);                  // repaint window.
       break;
-    case WM_SETFOCUS: 
+    case WM_SETFOCUS:
       SetFocus(client_status_edit_box);
       break;
     case WM_PAINT:
@@ -501,7 +501,7 @@ int APIENTRY _tWinMain(HINSTANCE instance,
   MyRegisterClass(instance);
 
   // Perform application initialization.
-  if (!InitInstance (instance, command_show)) {
+  if (!InitInstance(instance, command_show)) {
     return FALSE;
   }
 
@@ -518,5 +518,5 @@ int APIENTRY _tWinMain(HINSTANCE instance,
     }
   }
 
-  return (int)msg.wParam;
+  return static_cast<int>(msg.wParam);
 }
diff --git a/src/client/windows/unittests/exception_handler_death_test.cc b/src/client/windows/unittests/exception_handler_death_test.cc
index ba7be920..3a16e525 100644
--- a/src/client/windows/unittests/exception_handler_death_test.cc
+++ b/src/client/windows/unittests/exception_handler_death_test.cc
@@ -124,17 +124,19 @@ TEST_F(ExceptionHandlerDeathTest, InProcTest) {
   // the semantics of the exception handler being inherited/not
   // inherited across CreateProcess().
   ASSERT_TRUE(DoesPathExist(temp_path_));
-  google_breakpad::ExceptionHandler *exc =
-    new google_breakpad::ExceptionHandler(
-    temp_path_, NULL, &MinidumpWrittenCallback, NULL,
-    google_breakpad::ExceptionHandler::HANDLER_ALL);
+  scoped_ptr<google_breakpad::ExceptionHandler> exc(
+      new google_breakpad::ExceptionHandler(
+          temp_path_,
+          NULL,
+          &MinidumpWrittenCallback,
+          NULL,
+          google_breakpad::ExceptionHandler::HANDLER_ALL));
 
   // Disable GTest SEH handler
   testing::DisableExceptionHandlerInScope disable_exception_handler;
 
   int *i = NULL;
   ASSERT_DEATH((*i)++, kSuccessIndicator);
-  delete exc;
 }
 
 static bool gDumpCallbackCalled = false;
@@ -147,26 +149,24 @@ void clientDumpCallback(void *dump_context,
 
 void ExceptionHandlerDeathTest::DoCrashAccessViolation(
     const OutOfProcGuarantee out_of_proc_guarantee) {
-  google_breakpad::ExceptionHandler *exc = NULL;
+  scoped_ptr<google_breakpad::ExceptionHandler> exc;
 
   if (out_of_proc_guarantee == OUT_OF_PROC_GUARANTEED) {
-    google_breakpad::CrashGenerationClient *client = 
+    google_breakpad::CrashGenerationClient *client =
         new google_breakpad::CrashGenerationClient(kPipeName,
                                                    MiniDumpNormal,
                                                    NULL);  // custom_info
     ASSERT_TRUE(client->Register());
-    exc = new google_breakpad::ExceptionHandler(
+    exc.reset(new google_breakpad::ExceptionHandler(
         temp_path_,
         NULL,   // filter
         NULL,   // callback
         NULL,   // callback_context
         google_breakpad::ExceptionHandler::HANDLER_ALL,
-        MiniDumpNormal,
-        client,
-        NULL);  // custom_info
+        client));
   } else {
     ASSERT_TRUE(out_of_proc_guarantee == OUT_OF_PROC_BEST_EFFORT);
-    exc = new google_breakpad::ExceptionHandler(
+    exc.reset(new google_breakpad::ExceptionHandler(
         temp_path_,
         NULL,   // filter
         NULL,   // callback
@@ -174,7 +174,7 @@ void ExceptionHandlerDeathTest::DoCrashAccessViolation(
         google_breakpad::ExceptionHandler::HANDLER_ALL,
         MiniDumpNormal,
         kPipeName,
-        NULL);  // custom_info
+        NULL));  // custom_info
   }
 
   // Disable GTest SEH handler
@@ -302,17 +302,20 @@ wstring find_minidump_in_directory(const wstring &directory) {
       filename = directory + L"\\" + find_data.cFileName;
       break;
     }
-  } while(FindNextFile(find_handle, &find_data));
+  } while (FindNextFile(find_handle, &find_data));
   FindClose(find_handle);
   return filename;
 }
 
 TEST_F(ExceptionHandlerDeathTest, InstructionPointerMemory) {
   ASSERT_TRUE(DoesPathExist(temp_path_));
-  google_breakpad::ExceptionHandler *exc =
+  scoped_ptr<google_breakpad::ExceptionHandler> exc(
       new google_breakpad::ExceptionHandler(
-          temp_path_, NULL, NULL, NULL,
-          google_breakpad::ExceptionHandler::HANDLER_ALL);
+          temp_path_,
+          NULL,
+          NULL,
+          NULL,
+          google_breakpad::ExceptionHandler::HANDLER_ALL));
 
   // Disable GTest SEH handler
   testing::DisableExceptionHandlerInScope disable_exception_handler;
@@ -333,7 +336,7 @@ TEST_F(ExceptionHandlerDeathTest, InstructionPointerMemory) {
   // minidump should contain 128 bytes on either side of the
   // instruction pointer.
   memcpy(memory + kOffset, instructions, sizeof(instructions));
-  
+
   // Now execute the instructions, which should crash.
   typedef void (*void_function)(void);
   void_function memory_function =
@@ -382,11 +385,10 @@ TEST_F(ExceptionHandlerDeathTest, InstructionPointerMemory) {
     uint8_t suffix_bytes[kMemorySize - kOffset - sizeof(instructions)];
     memset(prefix_bytes, 0, sizeof(prefix_bytes));
     memset(suffix_bytes, 0, sizeof(suffix_bytes));
-    EXPECT_TRUE(memcmp(bytes, prefix_bytes, sizeof(prefix_bytes)) == 0);
-    EXPECT_TRUE(memcmp(bytes + kOffset, instructions,
-                       sizeof(instructions)) == 0);
-    EXPECT_TRUE(memcmp(bytes + kOffset + sizeof(instructions),
-                       suffix_bytes, sizeof(suffix_bytes)) == 0);
+    EXPECT_EQ(0, memcmp(bytes, prefix_bytes, sizeof(prefix_bytes)));
+    EXPECT_EQ(0, memcmp(bytes + kOffset, instructions, sizeof(instructions)));
+    EXPECT_EQ(0, memcmp(bytes + kOffset + sizeof(instructions),
+                        suffix_bytes, sizeof(suffix_bytes)));
   }
 
   DeleteFileW(minidump_filename_wide.c_str());
@@ -394,10 +396,13 @@ TEST_F(ExceptionHandlerDeathTest, InstructionPointerMemory) {
 
 TEST_F(ExceptionHandlerDeathTest, InstructionPointerMemoryMinBound) {
   ASSERT_TRUE(DoesPathExist(temp_path_));
-  google_breakpad::ExceptionHandler *exc =
+  scoped_ptr<google_breakpad::ExceptionHandler> exc(
       new google_breakpad::ExceptionHandler(
-          temp_path_, NULL, NULL, NULL,
-          google_breakpad::ExceptionHandler::HANDLER_ALL);
+          temp_path_,
+          NULL,
+          NULL,
+          NULL,
+          google_breakpad::ExceptionHandler::HANDLER_ALL));
 
   // Disable GTest SEH handler
   testing::DisableExceptionHandlerInScope disable_exception_handler;
@@ -426,7 +431,7 @@ TEST_F(ExceptionHandlerDeathTest, InstructionPointerMemoryMinBound) {
   // minidump should contain 128 bytes on either side of the
   // instruction pointer.
   memcpy(memory + kOffset, instructions, sizeof(instructions));
-  
+
   // Now execute the instructions, which should crash.
   typedef void (*void_function)(void);
   void_function memory_function =
@@ -484,10 +489,13 @@ TEST_F(ExceptionHandlerDeathTest, InstructionPointerMemoryMinBound) {
 
 TEST_F(ExceptionHandlerDeathTest, InstructionPointerMemoryMaxBound) {
   ASSERT_TRUE(DoesPathExist(temp_path_));
-  google_breakpad::ExceptionHandler *exc =
+  scoped_ptr<google_breakpad::ExceptionHandler> exc(
       new google_breakpad::ExceptionHandler(
-          temp_path_, NULL, NULL, NULL,
-          google_breakpad::ExceptionHandler::HANDLER_ALL);
+          temp_path_,
+          NULL,
+          NULL,
+          NULL,
+          google_breakpad::ExceptionHandler::HANDLER_ALL));
 
   // Disable GTest SEH handler
   testing::DisableExceptionHandlerInScope disable_exception_handler;
@@ -511,7 +519,7 @@ TEST_F(ExceptionHandlerDeathTest, InstructionPointerMemoryMaxBound) {
 
   // Write some instructions that will crash.
   memcpy(memory + kOffset, instructions, sizeof(instructions));
-  
+
   // Now execute the instructions, which should crash.
   typedef void (*void_function)(void);
   void_function memory_function =
@@ -559,9 +567,9 @@ TEST_F(ExceptionHandlerDeathTest, InstructionPointerMemoryMaxBound) {
 
     uint8_t prefix_bytes[kPrefixSize];
     memset(prefix_bytes, 0, sizeof(prefix_bytes));
-    EXPECT_TRUE(memcmp(bytes, prefix_bytes, sizeof(prefix_bytes)) == 0);
-    EXPECT_TRUE(memcmp(bytes + kPrefixSize,
-                       instructions, sizeof(instructions)) == 0);
+    EXPECT_EQ(0, memcmp(bytes, prefix_bytes, sizeof(prefix_bytes)));
+    EXPECT_EQ(0, memcmp(bytes + kPrefixSize,
+                        instructions, sizeof(instructions)));
   }
 
   DeleteFileW(minidump_filename_wide.c_str());
diff --git a/src/client/windows/unittests/exception_handler_nesting_test.cc b/src/client/windows/unittests/exception_handler_nesting_test.cc
index 0cf30013..3ae1d7cd 100755
--- a/src/client/windows/unittests/exception_handler_nesting_test.cc
+++ b/src/client/windows/unittests/exception_handler_nesting_test.cc
@@ -149,7 +149,7 @@ void InstallExceptionHandlerAndCrash(bool install_filter,
   ASSERT_TRUE(DoesPathExist(temp_path));
   google_breakpad::ExceptionHandler exc(
       temp_path,
-      install_filter ? 
+      install_filter ?
         (filter_return_value ?
           &CrashHandlerFilter<true> :
           &CrashHandlerFilter<false>) :
@@ -178,7 +178,7 @@ TEST(AssertDeathSanity, Regex) {
       std::string(kFoo) +
       std::string(kEndOfLine));
 
-  ASSERT_DEATH(DoCrash(kBar), 
+  ASSERT_DEATH(DoCrash(kBar),
     std::string(kStartOfLine) +
       std::string(kBar) +
       std::string(kEndOfLine));
diff --git a/src/client/windows/unittests/exception_handler_test.cc b/src/client/windows/unittests/exception_handler_test.cc
index 7b50e301..55275323 100644
--- a/src/client/windows/unittests/exception_handler_test.cc
+++ b/src/client/windows/unittests/exception_handler_test.cc
@@ -390,7 +390,7 @@ TEST_F(ExceptionHandlerTest, WriteMinidumpTest) {
   // Read the minidump and verify some info.
   Minidump minidump(minidump_filename);
   ASSERT_TRUE(minidump.Read());
-  //TODO(ted): more comprehensive tests...
+  // TODO(ted): more comprehensive tests...
 }
 
 // Test that an additional memory region can be included in the minidump.
diff --git a/src/client/windows/unittests/minidump_test.cc b/src/client/windows/unittests/minidump_test.cc
index 62f212f8..e3097f28 100644
--- a/src/client/windows/unittests/minidump_test.cc
+++ b/src/client/windows/unittests/minidump_test.cc
@@ -161,7 +161,8 @@ bool HasFileInfo(const std::wstring& file_path) {
 }
 
 TEST_F(MinidumpTest, Version) {
-  API_VERSION* version = ::ImagehlpApiVersion();
+  // Loads DbgHelp.dll in process
+  ImagehlpApiVersion();
 
   HMODULE dbg_help = ::GetModuleHandle(L"dbghelp.dll");
   ASSERT_TRUE(dbg_help != NULL);