Implement File::WriteAtCurrentPos for Windows

base/files/file_win.cc

Index: base/files/file_win.cc
diff --git a/base/files/file_win.cc b/base/files/file_win.cc
index 3581fca723077d7bc72822c937da5e3d647d7ebd..2bd69c7955595fec5ab9036a17f84e8cf9ed8c1c 100644
--- a/base/files/file_win.cc
+++ b/base/files/file_win.cc
@@ -139,7 +139,7 @@ int File::Read(int64 offset, char* data, int size) {
   overlapped.OffsetHigh = offset_li.HighPart;
 
   DWORD bytes_read;
-  if (::ReadFile(file_, data, size, &bytes_read, &overlapped) != 0)
+  if (::ReadFile(file_, data, size, &bytes_read, &overlapped))

[cpu_(ooo_6.6-7.5), 2014/03/06 20:43:41]

I am confused about the style unification here, below you seem to want

 if ( WindowsCallThatReuturnsBOOL(...) != FALSE)

But at 142 you dont.

[rvargas (doing something else), 2014/03/06 21:55:34]

I generally don't want to compare against anything (like here), but there are
some places (return statements) where we have to compare against something or
the compiler will warn about casting from int to bool (and fail given that there
is a warning). Between comparing against FALSE and using "!!Foo()" I prefer
comparing against FALSE.

     return bytes_read;
   if (ERROR_HANDLE_EOF == GetLastError())
     return 0;
@@ -155,7 +155,7 @@ int File::ReadAtCurrentPos(char* data, int size) {
     return -1;
 
   DWORD bytes_read;
-  if (::ReadFile(file_, data, size, &bytes_read, NULL) != 0)
+  if (::ReadFile(file_, data, size, &bytes_read, NULL))
     return bytes_read;
   if (ERROR_HANDLE_EOF == GetLastError())
     return 0;
@@ -184,14 +184,23 @@ int File::Write(int64 offset, const char* data, int size) {
   overlapped.OffsetHigh = offset_li.HighPart;
 
   DWORD bytes_written;
-  if (::WriteFile(file_, data, size, &bytes_written, &overlapped) != 0)
+  if (::WriteFile(file_, data, size, &bytes_written, &overlapped))
     return bytes_written;
 
   return -1;
 }
 
 int File::WriteAtCurrentPos(const char* data, int size) {

[Nico, 2014/03/06 23:57:59]

fwiw, i'd have named "write" "writeatoffset" and "writeatcurrentpos" "write",
for consistentcy with fwrite and istream::write etc

-  NOTREACHED();
+  base::ThreadRestrictions::AssertIOAllowed();
+  DCHECK(IsValid());
+  DCHECK(!async_);
+  if (size < 0)
+    return -1;
+
+  DWORD bytes_written;
+  if (::WriteFile(file_, data, size, &bytes_written, NULL))
+    return bytes_written;
+
   return -1;
 }
 
@@ -217,7 +226,7 @@ bool File::SetLength(int64 length) {
   LARGE_INTEGER file_pointer;
   LARGE_INTEGER zero;
   zero.QuadPart = 0;
-  if (::SetFilePointerEx(file_, zero, &file_pointer, FILE_CURRENT) == 0)
+  if (!::SetFilePointerEx(file_, zero, &file_pointer, FILE_CURRENT))
     return false;
 
   LARGE_INTEGER length_li;
@@ -233,8 +242,8 @@ bool File::SetLength(int64 length) {
   // TODO(rvargas): Emulating ftruncate details seem suspicious and it is not
   // promised by the interface (nor was promised by PlatformFile). See if this
   // implementation detail can be removed.
-  return ((::SetEndOfFile(file_) != 0) &&
-          (::SetFilePointerEx(file_, file_pointer, NULL, FILE_BEGIN) != 0));
+  return ((::SetEndOfFile(file_) != FALSE) &&
+          (::SetFilePointerEx(file_, file_pointer, NULL, FILE_BEGIN) != FALSE));
 }
 
 bool File::Flush() {
@@ -250,7 +259,7 @@ bool File::SetTimes(Time last_access_time, Time last_modified_time) {
   FILETIME last_access_filetime = last_access_time.ToFileTime();
   FILETIME last_modified_filetime = last_modified_time.ToFileTime();
   return (::SetFileTime(file_, NULL, &last_access_filetime,
-                        &last_modified_filetime) != 0);
+                        &last_modified_filetime) != FALSE);
 }
 
 bool File::GetInfo(Info* info) {
@@ -258,7 +267,7 @@ bool File::GetInfo(Info* info) {
   DCHECK(IsValid());
 
   BY_HANDLE_FILE_INFORMATION file_info;
-  if (GetFileInformationByHandle(file_, &file_info) == 0)
+  if (!GetFileInformationByHandle(file_, &file_info))
     return false;
 
   LARGE_INTEGER size;