Tweaks to SharedMemory to avoid CHECK when printing

base/memory/shared_memory_win.cc

Index: base/memory/shared_memory_win.cc
diff --git a/base/memory/shared_memory_win.cc b/base/memory/shared_memory_win.cc
index 5f706fe648598642584903f39a5aa40786acc88d..3c9f4b77d08710f407f2c6fb9406bf07454a1a5d 100644
--- a/base/memory/shared_memory_win.cc
+++ b/base/memory/shared_memory_win.cc
@@ -74,7 +74,7 @@ SharedMemory::~SharedMemory() {
 
 // static
 bool SharedMemory::IsHandleValid(const SharedMemoryHandle& handle) {
-  return handle != NULL;
+  return handle != NULL && handle != INVALID_HANDLE_VALUE;

[rvargas (doing something else), 2015/09/18 22:36:25]

<rant>
I know I'm grumpy, but this pattern doesn't make sense with or without this
change :(.

I know this is not what this method is promising, but there's no way to see if a
random handle is valid, and I'd rather remove this method completely and leave
that job to whoever is creating the handle (aka, remove raw handles completely).

Now, I understand that fix would be out of the scope of the current problem.

If we go with the current API, if a caller grabs a number from a hat and uses
it, it's basically depending on the internal implementation of this class, so
widening the range of values that we won't complain about is... kind of silly? I
hear that we should make this less prone to misuse, but really, we should fix
the API instead of adding knee pads to the cutting edges. We should be using
ScopedHandles here, and the caller should be using ScopedHandles, even if we end
up passing the raw handle through the interface.
</rant>

 }
 
 // static
@@ -84,7 +84,7 @@ SharedMemoryHandle SharedMemory::NULLHandle() {
 
 // static
 void SharedMemory::CloseHandle(const SharedMemoryHandle& handle) {
-  DCHECK(handle != NULL);
+  DCHECK(IsHandleValid(handle));

[rvargas (doing something else), 2015/09/18 22:36:25]

ScopedHandle closer(handle);

<rant> receiving const SharedHandle& for something that we'll be essentially
destroying doesn't make sense to me. This method also has to go away </rant>

   ::CloseHandle(handle);
 }
 
@@ -178,14 +178,14 @@ bool SharedMemory::Delete(const std::string& name) {
 }
 
 bool SharedMemory::Open(const std::string& name, bool read_only) {
-  DCHECK(!mapped_file_);
+  DCHECK(!IsHandleValid(mapped_file_));

[rvargas (doing something else), 2015/09/18 22:36:25]

DCHECK(mapped_file_.Isvalid());

(+ making the member a ScopedHandle). etc.

 
   name_ = ASCIIToUTF16(name);
   read_only_ = read_only;
   mapped_file_ = OpenFileMapping(
       read_only_ ? FILE_MAP_READ : FILE_MAP_READ | FILE_MAP_WRITE,
       false, name_.empty() ? NULL : name_.c_str());
-  if (mapped_file_ != NULL) {
+  if (IsHandleValid(mapped_file_)) {
     // Note: size_ is not set in this case.
     return true;
   }
@@ -193,7 +193,7 @@ bool SharedMemory::Open(const std::string& name, bool read_only) {
 }
 
 bool SharedMemory::MapAt(off_t offset, size_t bytes) {
-  if (mapped_file_ == NULL)
+  if (!IsHandleValid(mapped_file_))
     return false;
 
   if (bytes > static_cast<size_t>(std::numeric_limits<int>::max()))
@@ -259,7 +259,7 @@ bool SharedMemory::ShareToProcessCommon(ProcessHandle process,
 
 
 void SharedMemory::Close() {
-  if (mapped_file_ != NULL) {
+  if (IsHandleValid(mapped_file_)) {
     CloseHandle(mapped_file_);
     mapped_file_ = NULL;
   }