Fix CloseHandle blunder. I was thinking that the handle would be closed by S...

base/test_file_util_win.cc

 // Copyright (c) 2008 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/test_file_util.h"
 
 #include <windows.h>
 
 #include <vector>
 
@@ skipping 66 matching lines @@
 
     // If this is false, then we just processed the last portion of the file.
     if (!file_is_page_aligned)
       break;
   }
 
   if (!file_is_page_aligned) {
     // The size of the file isn't a multiple of the page size, so we'll have
     // to open the file again, this time without the FILE_FLAG_NO_BUFFERING
     // flag and use SetEndOfFile to mark EOF.
+    file_handle.Set(NULL);
     file_handle.Set(CreateFile(file, GENERIC_WRITE, 0, NULL, OPEN_EXISTING,

[darin (slow to review), 2008/12/08 18:18:44]

maybe the smart pointer class should be made smarter?  this seems like something
that could easily be misused again in the future, and it would suck if that were
actually in production code!

[tommi (sloooow) - chröme, 2008/12/09 10:07:45]

in this case I think it was just the engineer that should have been smarter ;)
although (arguably), the Set() method might be too prone to this kind of thing. 
When I tested the code, I was using ATL's CHandle class and not ScopedHandle. 
CHandle exposes the Close() method, which is what I called.  Close() is private
for ScopeHandle, so I couldn't call it and when I then switched out CHandle for
ScopeHandle, I thought "oh, Set() already closes the handle for me, so all I
need is Set()"... big mistake.
Since handles are slightly different from regular memory pointers (i.e. no
mutual exclusion) it might be better to have no [re]Set() method, but instead
have Close() and Attach() where Attach() only DCHECKs that it's not already
valid.  That way we get away with as little generated code as possible and avoid
cases like this.  In this particular case, since I have to use Set(), two
possible calls to CloseHandle will be generated but only one will ever execute. 
That's a few wasted bytes :)

                                0, NULL));
     CHECK(SetFilePointer(file_handle, total_bytes, NULL, FILE_BEGIN) !=
           INVALID_SET_FILE_POINTER);
     CHECK(::SetEndOfFile(file_handle));
   }
 
   // Restore the file attributes.
   CHECK(::SetFileTime(file_handle, &bhi.ftCreationTime, &bhi.ftLastAccessTime,
                       &bhi.ftLastWriteTime));
 
@@ skipping 51 matching lines @@
       EvictFileFromSystemCache(cur_dest_path.c_str());
     }
   } while (FindNextFile(fh, &fd));
 
   FindClose(fh);
   return true;
 }
 
 }  // namespace file_util