Fixes for re-enabling more MSVC level 4 warnings: sandbox/ edition

This contains fixes for the following sorts of issues:
* Assignment inside conditional
* Possibly-uninitialized local variable
* Signedness mismatch

This also contains a small number of other cleanups/simplifications to
nearby code.

BUG=81439
TEST=none

Review URL: https://codereview.chromium.org/382613002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@282451 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 47 insertions, 58 deletions

diff --git a/sandbox/win/src/handle_closer_test.cc b/sandbox/win/src/handle_closer_test.cc
index 9adcf6c..8e821ce 100644
--- a/sandbox/win/src/handle_closer_test.cc
+++ b/sandbox/win/src/handle_closer_test.cc
@@ -157,7 +157,8 @@ void WINAPI ThreadPoolTask(void* event, BOOLEAN timeout) {
 // Run a thread pool inside a sandbox without a CSRSS connection.
 SBOX_TESTS_COMMAND int RunThreadPool(int argc, wchar_t **argv) {
   HANDLE wait_list[20];
-  CHECK(finish_event = ::CreateEvent(NULL, TRUE, FALSE, NULL));
+  finish_event = ::CreateEvent(NULL, TRUE, FALSE, NULL);
+  CHECK(finish_event);
 
   // Set up a bunch of waiters.
   HANDLE pool = NULL;
diff --git a/sandbox/win/src/handle_dispatcher.cc b/sandbox/win/src/handle_dispatcher.cc
index 6acb6f9..fda7aac 100644
--- a/sandbox/win/src/handle_dispatcher.cc
+++ b/sandbox/win/src/handle_dispatcher.cc
@@ -44,7 +44,6 @@ bool HandleDispatcher::DuplicateHandleProxy(IPCInfo* ipc,
                                             DWORD target_process_id,
                                             DWORD desired_access,
                                             DWORD options) {
-  NTSTATUS error;
   static NtQueryObject QueryObject = NULL;
   if (!QueryObject)
     ResolveNTFunctionPtr("NtQueryObject", &QueryObject);
@@ -65,9 +64,10 @@ bool HandleDispatcher::DuplicateHandleProxy(IPCInfo* ipc,
   OBJECT_TYPE_INFORMATION* type_info =
       reinterpret_cast<OBJECT_TYPE_INFORMATION*>(buffer);
   ULONG size = sizeof(buffer) - sizeof(wchar_t);
-  error = QueryObject(handle, ObjectTypeInformation, type_info, size, &size);
+  NTSTATUS error =
+      QueryObject(handle, ObjectTypeInformation, type_info, size, &size);
   if (!NT_SUCCESS(error)) {
-    ipc->return_info.win32_result = error;
+    ipc->return_info.nt_status = error;
     return false;
   }
   type_info->Name.Buffer[type_info->Name.Length / sizeof(wchar_t)] = L'\0';
diff --git a/sandbox/win/src/handle_interception.cc b/sandbox/win/src/handle_interception.cc
index af0bebc..a0df8d6 100644
--- a/sandbox/win/src/handle_interception.cc
+++ b/sandbox/win/src/handle_interception.cc
@@ -33,7 +33,7 @@ ResultCode DuplicateHandleProxy(HANDLE source_handle,
     return code;
 
   if (answer.win32_result) {
-    ::SetLastError(answer.nt_status);
+    ::SetLastError(answer.win32_result);
     return SBOX_ERROR_GENERIC;
   }
 
diff --git a/sandbox/win/src/process_policy_test.cc b/sandbox/win/src/process_policy_test.cc
index af64f14..ae62606 100644
--- a/sandbox/win/src/process_policy_test.cc
+++ b/sandbox/win/src/process_policy_test.cc
@@ -154,28 +154,20 @@ SBOX_TESTS_COMMAND int Process_RunApp4(int argc, wchar_t **argv) {
   // TEST 4: Try file name in the app_name and current directory sets correctly.
   base::string16 system32 = MakeFullPathToSystem32(L"");
   wchar_t current_directory[MAX_PATH + 1];
-  int result4;
-  bool test_succeeded = false;
   DWORD ret = ::GetCurrentDirectory(MAX_PATH, current_directory);
   if (!ret)
     return SBOX_TEST_FIRST_ERROR;
+  if (ret >= MAX_PATH)
+    return SBOX_TEST_FAILED;
 
-  if (ret < MAX_PATH) {
-    current_directory[ret] = L'\\';
-    current_directory[ret+1] = L'\0';
-    if (::SetCurrentDirectory(system32.c_str())) {
-      result4 = CreateProcessHelper(argv[0], base::string16());
-      if (::SetCurrentDirectory(current_directory)) {
-        test_succeeded = true;
-      }
-    } else {
-      return SBOX_TEST_SECOND_ERROR;
-    }
+  current_directory[ret] = L'\\';
+  current_directory[ret+1] = L'\0';
+  if (!::SetCurrentDirectory(system32.c_str())) {
+    return SBOX_TEST_SECOND_ERROR;
   }
-  if (!test_succeeded)
-    result4 = SBOX_TEST_FAILED;
 
-  return result4;
+  const int result4 = CreateProcessHelper(argv[0], base::string16());
+  return ::SetCurrentDirectory(current_directory) ? result4 : SBOX_TEST_FAILED;
 }
 
 SBOX_TESTS_COMMAND int Process_RunApp5(int argc, wchar_t **argv) {
diff --git a/sandbox/win/src/sync_dispatcher.cc b/sandbox/win/src/sync_dispatcher.cc
index d4b36d5..18a0743 100644
--- a/sandbox/win/src/sync_dispatcher.cc
+++ b/sandbox/win/src/sync_dispatcher.cc
@@ -35,12 +35,11 @@ SyncDispatcher::SyncDispatcher(PolicyBase* policy_base)
 
 bool SyncDispatcher::SetupService(InterceptionManager* manager,
                                   int service) {
-  if (IPC_CREATEEVENT_TAG == service) {
+  if (service == IPC_CREATEEVENT_TAG) {
     return INTERCEPT_NT(manager, NtCreateEvent, CREATE_EVENT_ID, 24);
-  } else if (IPC_OPENEVENT_TAG == service) {
-    return INTERCEPT_NT(manager, NtOpenEvent, OPEN_EVENT_ID, 16);
   }
-  return false;
+  return (service == IPC_OPENEVENT_TAG) &&
+      INTERCEPT_NT(manager, NtOpenEvent, OPEN_EVENT_ID, 16);
 }
 
 bool SyncDispatcher::CreateEvent(IPCInfo* ipc, base::string16* name,
@@ -52,11 +51,9 @@ bool SyncDispatcher::CreateEvent(IPCInfo* ipc, base::string16* name,
   EvalResult result = policy_base_->EvalPolicy(IPC_CREATEEVENT_TAG,
                                                params.GetBase());
   HANDLE handle = NULL;
-  DWORD ret = SyncPolicy::CreateEventAction(result, *ipc->client_info, *name,
-                                            event_type, initial_state,
-                                            &handle);
   // Return operation status on the IPC.
-  ipc->return_info.nt_status = ret;
+  ipc->return_info.nt_status = SyncPolicy::CreateEventAction(
+      result, *ipc->client_info, *name, event_type, initial_state, &handle);
   ipc->return_info.handle = handle;
   return true;
 }
@@ -72,10 +69,9 @@ bool SyncDispatcher::OpenEvent(IPCInfo* ipc, base::string16* name,
   EvalResult result = policy_base_->EvalPolicy(IPC_OPENEVENT_TAG,
                                                params.GetBase());
   HANDLE handle = NULL;
-  DWORD ret = SyncPolicy::OpenEventAction(result, *ipc->client_info, *name,
-                                          desired_access, &handle);
   // Return operation status on the IPC.
-  ipc->return_info.win32_result = ret;
+  ipc->return_info.nt_status = SyncPolicy::OpenEventAction(
+      result, *ipc->client_info, *name, desired_access, &handle);
   ipc->return_info.handle = handle;
   return true;
 }
diff --git a/sandbox/win/src/sync_policy.cc b/sandbox/win/src/sync_policy.cc
index 7b18fe7..75399e6 100644
--- a/sandbox/win/src/sync_policy.cc
+++ b/sandbox/win/src/sync_policy.cc
@@ -176,12 +176,12 @@ bool SyncPolicy::GenerateRules(const wchar_t* name,
   return true;
 }
 
-DWORD SyncPolicy::CreateEventAction(EvalResult eval_result,
-                                    const ClientInfo& client_info,
-                                    const base::string16 &event_name,
-                                    uint32 event_type,
-                                    uint32 initial_state,
-                                    HANDLE *handle) {
+NTSTATUS SyncPolicy::CreateEventAction(EvalResult eval_result,
+                                       const ClientInfo& client_info,
+                                       const base::string16 &event_name,
+                                       uint32 event_type,
+                                       uint32 initial_state,
+                                       HANDLE *handle) {
   NtCreateEventFunction NtCreateEvent = NULL;
   ResolveNTFunctionPtr("NtCreateEvent", &NtCreateEvent);
 
@@ -214,11 +214,11 @@ DWORD SyncPolicy::CreateEventAction(EvalResult eval_result,
   return status;
 }
 
-DWORD SyncPolicy::OpenEventAction(EvalResult eval_result,
-                                  const ClientInfo& client_info,
-                                  const base::string16 &event_name,
-                                  uint32 desired_access,
-                                  HANDLE *handle) {
+NTSTATUS SyncPolicy::OpenEventAction(EvalResult eval_result,
+                                     const ClientInfo& client_info,
+                                     const base::string16 &event_name,
+                                     uint32 desired_access,
+                                     HANDLE *handle) {
   NtOpenEventFunction NtOpenEvent = NULL;
   ResolveNTFunctionPtr("NtOpenEvent", &NtOpenEvent);
 
diff --git a/sandbox/win/src/sync_policy.h b/sandbox/win/src/sync_policy.h
index 4383998..e370e4b 100644
--- a/sandbox/win/src/sync_policy.h
+++ b/sandbox/win/src/sync_policy.h
@@ -33,17 +33,17 @@ class SyncPolicy {
   // Performs the desired policy action on a request.
   // client_info is the target process that is making the request and
   // eval_result is the desired policy action to accomplish.
-  static DWORD CreateEventAction(EvalResult eval_result,
-                                 const ClientInfo& client_info,
-                                 const base::string16 &event_name,
-                                 uint32 event_type,
-                                 uint32 initial_state,
-                                 HANDLE *handle);
-  static DWORD OpenEventAction(EvalResult eval_result,
-                               const ClientInfo& client_info,
-                               const base::string16 &event_name,
-                               uint32 desired_access,
-                               HANDLE *handle);
+  static NTSTATUS CreateEventAction(EvalResult eval_result,
+                                    const ClientInfo& client_info,
+                                    const base::string16 &event_name,
+                                    uint32 event_type,
+                                    uint32 initial_state,
+                                    HANDLE *handle);
+  static NTSTATUS OpenEventAction(EvalResult eval_result,
+                                  const ClientInfo& client_info,
+                                  const base::string16 &event_name,
+                                  uint32 desired_access,
+                                  HANDLE *handle);
 };
 
 }  // namespace sandbox
diff --git a/sandbox/win/tests/common/controller.cc b/sandbox/win/tests/common/controller.cc
index ac20f6c..8d355b6 100644
--- a/sandbox/win/tests/common/controller.cc
+++ b/sandbox/win/tests/common/controller.cc
@@ -247,13 +247,13 @@ int TestRunner::InternalRunTest(const wchar_t* command) {
   }
 
   if (WAIT_TIMEOUT == ::WaitForSingleObject(target.hProcess, timeout_)) {
-    ::TerminateProcess(target.hProcess, SBOX_TEST_TIMED_OUT);
+    ::TerminateProcess(target.hProcess, static_cast<UINT>(SBOX_TEST_TIMED_OUT));
     ::CloseHandle(target.hProcess);
     ::CloseHandle(target.hThread);
     return SBOX_TEST_TIMED_OUT;
   }
 
-  DWORD exit_code = SBOX_TEST_LAST_RESULT;
+  DWORD exit_code = static_cast<DWORD>(SBOX_TEST_LAST_RESULT);
   if (!::GetExitCodeProcess(target.hProcess, &exit_code)) {
     ::CloseHandle(target.hProcess);
     ::CloseHandle(target.hThread);
diff --git a/sandbox/win/tests/validation_tests/commands.cc b/sandbox/win/tests/validation_tests/commands.cc
index 2613347..45717b4 100644
--- a/sandbox/win/tests/validation_tests/commands.cc
+++ b/sandbox/win/tests/validation_tests/commands.cc
@@ -276,8 +276,8 @@ int TestOpenAlternateDesktop(wchar_t *desktop_name) {
   }
 
   // Open by name with WRITE_DAC.
-  if ((desktop = ::OpenDesktop(desktop_name, 0, FALSE, WRITE_DAC)) ||
-      ::GetLastError() != ERROR_ACCESS_DENIED) {
+  desktop = ::OpenDesktop(desktop_name, 0, FALSE, WRITE_DAC);
+  if (desktop || ::GetLastError() != ERROR_ACCESS_DENIED) {
     ::CloseDesktop(desktop);
     return SBOX_TEST_SUCCEEDED;
   }