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

sandbox/win/src/handle_dispatcher.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "sandbox/win/src/handle_dispatcher.h"
 
 #include "base/win/scoped_handle.h"
 #include "sandbox/win/src/handle_interception.h"
 #include "sandbox/win/src/handle_policy.h"
 #include "sandbox/win/src/ipc_tags.h"
@@ skipping 26 matching lines @@
   }
 
   return false;
 }
 
 bool HandleDispatcher::DuplicateHandleProxy(IPCInfo* ipc,
                                             HANDLE source_handle,
                                             DWORD target_process_id,
                                             DWORD desired_access,
                                             DWORD options) {
-  NTSTATUS error;
   static NtQueryObject QueryObject = NULL;
   if (!QueryObject)
     ResolveNTFunctionPtr("NtQueryObject", &QueryObject);
 
   // Get a copy of the handle for use in the broker process.
   HANDLE handle_temp;
   if (!::DuplicateHandle(ipc->client_info->process, source_handle,
                          ::GetCurrentProcess(), &handle_temp,
                          0, FALSE, DUPLICATE_SAME_ACCESS | options)) {
     ipc->return_info.win32_result = ::GetLastError();
     return false;
   }
   options &= ~DUPLICATE_CLOSE_SOURCE;
   base::win::ScopedHandle handle(handle_temp);
 
   // Get the object type (32 characters is safe; current max is 14).
   BYTE buffer[sizeof(OBJECT_TYPE_INFORMATION) + 32 * sizeof(wchar_t)];
   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;

[Peter Kasting, 2014/07/09 18:55:18]

Note that because win32_result and nt_status are in a union, this doesn't
actually result in the sort of behavior change it might look like.  (Similar
comments apply elsewhere in this CL; in general, changes like this are attempts
to make sure we read and write the fields that match the types of the local
variables we're reading and writing to.)

[cpu_(ooo_6.6-7.5), 2014/07/10 00:49:34]

yeah that reads like it was a bug, but it can become one easily if one would
remove the union which its an optimization if my memory is correct.

[Peter Kasting, 2014/07/10 01:19:31]

I don't know if I understand this comment.

As far as I can tell, it was already the case that we sometimes wrote one format
and then read the other.  In other words, the union is not just for space
savings, it's actually functionally necessary (it effectively casts types back
and forth).

Given that, I don't know how to interpret what you're saying I should be doing
here.

     return false;
   }
   type_info->Name.Buffer[type_info->Name.Length / sizeof(wchar_t)] = L'\0';
 
   CountedParameterSet<HandleTarget> params;
   params[HandleTarget::NAME] = ParamPickerMake(type_info->Name.Buffer);
   params[HandleTarget::TARGET] = ParamPickerMake(target_process_id);
 
   EvalResult eval = policy_base_->EvalPolicy(IPC_DUPLICATEHANDLEPROXY_TAG,
                                              params.GetBase());
   ipc->return_info.win32_result =
       HandlePolicy::DuplicateHandleProxyAction(eval, handle,
                                                target_process_id,
                                                &ipc->return_info.handle,
                                                desired_access, options);
   return true;
 }
 
 }  // namespace sandbox