C++ reability review for gab.

chrome/installer/setup/setup_util.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.
 //
 // This file declares util functions for setup project.
 
+//Pickup file for readability review.
+
 #include "chrome/installer/setup/setup_util.h"
 
 #include <windows.h>
 
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/string_util.h"
 #include "chrome/installer/util/installer_state.h"
 #include "chrome/installer/util/util_constants.h"
 #include "courgette/courgette.h"
@@ skipping 64 matching lines @@
   }
 
   return (version_found ? max_version.release() : NULL);
 }
 
 bool DeleteFileFromTempProcess(const FilePath& path,
                                uint32 delay_before_delete_ms) {
   static const wchar_t kRunDll32Path[] =
       L"%SystemRoot%\\System32\\rundll32.exe";
   wchar_t rundll32[MAX_PATH];
-  DWORD size = ExpandEnvironmentStrings(kRunDll32Path, rundll32,
-                                        arraysize(rundll32));
+  DWORD size =
+      ExpandEnvironmentStrings(kRunDll32Path, rundll32, arraysize(rundll32));
   if (!size || size >= MAX_PATH)
     return false;
 
   STARTUPINFO startup = { sizeof(STARTUPINFO) };
   PROCESS_INFORMATION pi = {0};
   BOOL ok = ::CreateProcess(NULL, rundll32, NULL, NULL, FALSE, CREATE_SUSPENDED,
                             NULL, NULL, &startup, &pi);
   if (ok) {
     // We use the main thread of the new process to run:
     //   Sleep(delay_before_delete_ms);
     //   DeleteFile(path);
     //   ExitProcess(0);
     // This runs before the main routine of the process runs, so it doesn't
     // matter much which executable we choose except that we don't want to
     // use e.g. a console app that causes a window to be created.
     size = (path.value().length() + 1) * sizeof(path.value()[0]);
     void* mem = ::VirtualAllocEx(pi.hProcess, NULL, size, MEM_COMMIT,
                                  PAGE_READWRITE);
     if (mem) {
       SIZE_T written = 0;
-      ::WriteProcessMemory(pi.hProcess, mem, path.value().c_str(),
+      ::WriteProcessMemory(
+          pi.hProcess, mem, path.value().c_str(),
           (path.value().size() + 1) * sizeof(path.value()[0]), &written);
       HMODULE kernel32 = ::GetModuleHandle(L"kernel32.dll");
       PAPCFUNC sleep = reinterpret_cast<PAPCFUNC>(
           ::GetProcAddress(kernel32, "Sleep"));
       PAPCFUNC delete_file = reinterpret_cast<PAPCFUNC>(
           ::GetProcAddress(kernel32, "DeleteFileW"));
       PAPCFUNC exit_process = reinterpret_cast<PAPCFUNC>(
           ::GetProcAddress(kernel32, "ExitProcess"));
       if (!sleep || !delete_file || !exit_process) {
         NOTREACHED();
@@ skipping 18 matching lines @@
 
 string16 GetActiveSetupPath(BrowserDistribution* dist) {
   static const wchar_t kInstalledComponentsPath[] =
       L"Software\\Microsoft\\Active Setup\\Installed Components\\";
   return kInstalledComponentsPath + dist->GetAppGuid();
 }
 
 ScopedTokenPrivilege::ScopedTokenPrivilege(const wchar_t* privilege_name)
     : is_enabled_(false) {
   if (!::OpenProcessToken(::GetCurrentProcess(),
-      TOKEN_ADJUST_PRIVILEGES | TOKEN_QUERY, token_.Receive())) {
+                          TOKEN_ADJUST_PRIVILEGES | TOKEN_QUERY,
+                          token_.Receive())) {
     return;
   }
 
   LUID privilege_luid;
   if (!::LookupPrivilegeValue(NULL, privilege_name, &privilege_luid)) {
     token_.Close();
     return;
   }
 
   // Adjust the token's privileges to enable |privilege_name|. If this privilege
   // was already enabled, |previous_privileges_|.PrivilegeCount will be set to 0
   // and we then know not to disable this privilege upon destruction.
   TOKEN_PRIVILEGES tp;
   tp.PrivilegeCount = 1;
   tp.Privileges[0].Luid = privilege_luid;
   tp.Privileges[0].Attributes = SE_PRIVILEGE_ENABLED;
   DWORD return_length;
-  if (!::AdjustTokenPrivileges(token_, FALSE, &tp, sizeof(TOKEN_PRIVILEGES),
-                               &previous_privileges_, &return_length)) {
+  if (::AdjustTokenPrivileges(token_, FALSE, &tp, sizeof(TOKEN_PRIVILEGES),

[dominich, 2012/09/10 15:40:42]

this might be more readable as:

is_enabled_ = ::AdjustTokenPrivileges(token_, FALSE, &tp, 
                                      sizeof(TOKEN_PRIVILEGES),
                                      &previous_privileges_, &return_length);
if (!is_enabled_)
  token_.Close();

[gab, 2012/09/10 19:14:05]

Done.

+                              &previous_privileges_, &return_length)) {
+    is_enabled_ = true;
+  } else {
     token_.Close();
-  } else {
-    is_enabled_ = true;
   }
 }
 
 ScopedTokenPrivilege::~ScopedTokenPrivilege() {
   if (is_enabled_ && previous_privileges_.PrivilegeCount != 0) {
     ::AdjustTokenPrivileges(token_, FALSE, &previous_privileges_,
                             sizeof(TOKEN_PRIVILEGES), NULL, NULL);
   }
 }
 
 }  // namespace installer