Refactor AppendToFile and WriteFileDescriptor

base/files/file_util_win.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.
 
 #include "base/files/file_util.h"
 
 #include <windows.h>
 #include <io.h>
 #include <psapi.h>
 #include <shellapi.h>
@@ skipping 626 matching lines @@
     DPLOG(WARNING) << "writing file " << UTF16ToUTF8(filename.value())
                    << " failed";
   } else {
     // Didn't write all the bytes.
     DLOG(WARNING) << "wrote" << written << " bytes to "
                   << UTF16ToUTF8(filename.value()) << " expected " << size;
   }
   return -1;
 }
 
-int AppendToFile(const FilePath& filename, const char* data, int size) {
+bool AppendToFile(const FilePath& filename, const char* data, int size) {
   ThreadRestrictions::AssertIOAllowed();
   base::win::ScopedHandle file(CreateFile(filename.value().c_str(),
                                           FILE_APPEND_DATA,
                                           0,
                                           NULL,
                                           OPEN_EXISTING,
                                           0,
                                           NULL));
-  if (!file.IsValid()) {
-    DPLOG(WARNING) << "CreateFile failed for path "
-                   << UTF16ToUTF8(filename.value());
-    return -1;
+  if (!file.IsValid())
+    return false;
+
+  DWORD saved_error = 0;
+  DWORD written;
+
+  // According to MSFT documentation the only time a partial write can occur is
+  // if we are writing to a pipe, which we aren't currently doing so if this

[rvargas (doing something else), 2014/10/01 20:01:14]

I don't think this code has enough context to know if this is a pipe or not. In
any case, there is |written| already, so why not see what it is and return false
if != size?

+  // call succeeds we can assume that all the data was written out to the file.
+  if (!::WriteFile(file.Get(), data, size, &written, NULL))
+    saved_error = ::GetLastError();
+
+  // Close the file now so that we can restore the write error, if any.
+  file.Close();
+
+  // Restore any error that occurred while writing the data.  This takes
+  // precedence over any errors that happened while closing the file.
+  if (saved_error != 0) {
+    ::SetLastError(saved_error);
+    return false;
   }
 
-  DWORD written;
-  BOOL result = ::WriteFile(file.Get(), data, size, &written, NULL);
-  if (result && static_cast<int>(written) == size)
-    return written;
-
-  if (!result) {
-    // WriteFile failed.
-    DPLOG(WARNING) << "writing file " << UTF16ToUTF8(filename.value())
-                   << " failed";
-  } else {
-    // Didn't write all the bytes.
-    DLOG(WARNING) << "wrote" << written << " bytes to "
-                  << UTF16ToUTF8(filename.value()) << " expected " << size;
-  }
-  return -1;
+  return true;
 }
 
 // Gets the current working directory for the process.
 bool GetCurrentDirectory(FilePath* dir) {
   ThreadRestrictions::AssertIOAllowed();
 
   wchar_t system_buffer[MAX_PATH];
   system_buffer[0] = 0;
   DWORD len = ::GetCurrentDirectory(MAX_PATH, system_buffer);
   if (len == 0 || len > MAX_PATH)
@@ skipping 114 matching lines @@
     // Like Move, this function is not transactional, so we just
     // leave the copied bits behind if deleting from_path fails.
     // If to_path exists previously then we have already overwritten
     // it by now, we don't get better off by deleting the new bits.
   }
   return false;
 }
 
 }  // namespace internal
 }  // namespace base