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 125 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())) {

[dominich, 2012/09/06 17:26:12]

this should either align with ::GetCurrentProcess() or ::GetCurrentProcess
should be dropped down a line and all parameters should be indented 4 spaces.

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi...

[gab, 2012/09/07 20:56:44]

Done.

     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),

[dominich, 2012/09/06 17:26:12]

prefer the positive conditional first.

[gab, 2012/09/07 20:56:44]

Done.

                                &previous_privileges_, &return_length)) {
     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