Cleanup.

base/debug/gdi_debug_util_win.cc

 // Copyright 2014 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 "base/debug/gdi_debug_util_win.h"
 
 #include <cmath>
 
 #include <psapi.h>
 #include <TlHelp32.h>
 
 #include "base/debug/alias.h"
 #include "base/logging.h"
 #include "base/win/scoped_handle.h"
 
 namespace {
 
 void CollectChildGDIUsageAndDie(DWORD parent_pid) {
-  HANDLE snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0) ;
-  if(snapshot == INVALID_HANDLE_VALUE)
-    CHECK(false);
+  HANDLE snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);
+  CHECK_NE(INVALID_HANDLE_VALUE, snapshot);
 
   int child_count = 0;
   base::debug::Alias(&child_count);
   int peak_gdi_count = 0;
   base::debug::Alias(&peak_gdi_count);
   int sum_gdi_count = 0;
   base::debug::Alias(&sum_gdi_count);
   int sum_user_count = 0;
   base::debug::Alias(&sum_user_count);
 
   PROCESSENTRY32 proc_entry = {0};
-  proc_entry.dwSize = sizeof(PROCESSENTRY32) ;
-  if(!Process32First(snapshot, &proc_entry))
-    CHECK(false);
+  proc_entry.dwSize = sizeof(PROCESSENTRY32);
+  CHECK(Process32First(snapshot, &proc_entry));
 
   do {
     if (parent_pid != proc_entry.th32ParentProcessID)
       continue;
     // Got a child process. Compute GDI usage.
     base::win::ScopedHandle process(
         ::OpenProcess(PROCESS_QUERY_INFORMATION,
                       FALSE,
                       proc_entry.th32ParentProcessID));
     if (!process)
       continue;
 
     int num_gdi_handles = ::GetGuiResources(process.Get(), GR_GDIOBJECTS);
     int num_user_handles = ::GetGuiResources(process.Get(), GR_USEROBJECTS);
 
     // Compute sum and peak counts.
     ++child_count;
     sum_user_count += num_user_handles;
     sum_gdi_count += num_gdi_handles;
     if (peak_gdi_count < num_gdi_handles)
       peak_gdi_count = num_gdi_handles;
 
-  } while(Process32Next(snapshot, &proc_entry));
+  } while (Process32Next(snapshot, &proc_entry));
 

[cpu_(ooo_6.6-7.5), 2014/09/26 01:17:50]

there is an inconsistency in this file wrt windows api calls, the :: versus
nothing. I figure you might want to correct that while we are here.

[Peter Kasting, 2014/09/29 19:29:45]

Done.

-  ::CloseHandle(snapshot) ;
+  ::CloseHandle(snapshot);
   CHECK(false);

[cpu_(ooo_6.6-7.5), 2014/09/26 01:17:50]

I have no idea why we are closing the snapshot handle if we are going to go up
in flames. Did I coded that?

Anyhow remove the close.

[Peter Kasting, 2014/09/26 01:21:33]

It wasn't me :)
Is it really wrong?  If it's correct and just unnecessary I'd kinda rather leave
it in...

[cpu_(ooo_6.6-7.5), 2014/09/29 19:01:48]

Its best to not do cleanup if we are intent in generating a dump. We want to
preserve more state, not less.

Not hung on this, so fee free to skip. Just spelling the rationale.

[Peter Kasting, 2014/09/29 19:29:45]

OK.  I think I'll leave the code as-is rather than make a functional change
here.

 }
 
 }  // namespace
 
 namespace base {
 namespace debug {
 
 void GDIBitmapAllocFailure(BITMAPINFOHEADER* header, HANDLE shared_section) {
   // Make sure parameters are saved in the minidump.
   DWORD last_error = ::GetLastError();
@@ skipping 14 matching lines @@
   if (num_gdi_handles == 0) {
     DWORD get_gui_resources_error = GetLastError();
     base::debug::Alias(&get_gui_resources_error);
     CHECK(false);
   }
 
   base::debug::Alias(&num_gdi_handles);
   base::debug::Alias(&num_user_handles);
 
   const DWORD kLotsOfHandles = 9990;
-  if (num_gdi_handles > kLotsOfHandles)
-    CHECK(false);
+  CHECK_LE(num_gdi_handles, kLotsOfHandles);
 
   PROCESS_MEMORY_COUNTERS_EX pmc;
   pmc.cb = sizeof(pmc);
-  if (!GetProcessMemoryInfo(GetCurrentProcess(),
-                            reinterpret_cast<PROCESS_MEMORY_COUNTERS*>(&pmc),
-                            sizeof(pmc))) {
-    CHECK(false);
-  }
+  CHECK(GetProcessMemoryInfo(GetCurrentProcess(),
+                             reinterpret_cast<PROCESS_MEMORY_COUNTERS*>(&pmc),
+                             sizeof(pmc)));
   const size_t kLotsOfMemory = 1500 * 1024 * 1024; // 1.5GB
-  if (pmc.PagefileUsage > kLotsOfMemory)
-    CHECK(false);
-  if (pmc.PrivateUsage > kLotsOfMemory)
-    CHECK(false);
+  CHECK_LE(pmc.PagefileUsage, kLotsOfMemory);
+  CHECK_LE(pmc.PrivateUsage, kLotsOfMemory);
 
   void* small_data = NULL;
   base::debug::Alias(&small_data);
 
   if (std::abs(heigth) * width > 100) {
     // Huh, that's weird.  We don't have crazy handle count, we don't have
     // ridiculous memory usage. Try to allocate a small bitmap and see if that
     // fails too.
     header->biWidth = 5;
     header->biHeight = -5;
     HBITMAP small_bitmap = CreateDIBSection(
         NULL, reinterpret_cast<BITMAPINFO*>(&header),
         0, &small_data, shared_section, 0);
+    CHECK(small_bitmap != NULL);
+    DeleteObject(small_bitmap);
   }
   // Maybe the child processes are the ones leaking GDI or USER resouces.
   CollectChildGDIUsageAndDie(::GetCurrentProcessId());
 }
 
 }  // namespace debug
 }  // namespace base