Cross platform low level file IO wrappers

util/file/file_io_win.cc

Index: util/file/file_io_win.cc
diff --git a/util/numeric/in_range_cast.h b/util/file/file_io_win.cc
similarity index 39%
copy from util/numeric/in_range_cast.h
copy to util/file/file_io_win.cc
index 3dba10afa16016bb7149cd1fa7b4ae14c72cfca2..118ef7eca4164abcd93f3e2ab9169dd85c3ddcc8 100644
--- a/util/numeric/in_range_cast.h
+++ b/util/file/file_io_win.cc
@@ -12,33 +12,48 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#ifndef CRASHPAD_UTIL_NUMERIC_IN_RANGE_CAST_H_
-#define CRASHPAD_UTIL_NUMERIC_IN_RANGE_CAST_H_
+#include "util/file/file_io.h"
 
 #include "base/logging.h"
 #include "base/numerics/safe_conversions.h"
 
 namespace crashpad {
 
-//! \brief Casts to a different type if it can be done without data loss,
-//!     logging a warning message and returing a default value otherwise.
-//!
-//! \param[in] source The value to convert and return.
-//! \param[in] default_value The default value to return, in the event that \a
-//!     source cannot be represented in the destination type.
-//!
-//! \return \a source if it can be represented in the destination type,
-//!     otherwise \a default_value.
-template <typename Destination, typename Source>
-Destination InRangeCast(Source source, Destination default_value) {
-  if (base::IsValueInRangeForNumericType<Destination>(source)) {
-    return static_cast<Destination>(source);
+// TODO(scottmg): Handle > DWORD sized writes if necessary.
+
+ssize_t ReadFile(FileHandle file, void* buffer, size_t size) {
+  DWORD size_dword = base::checked_cast<DWORD>(size);
+  DWORD total_read = 0;
+  char* buffer_c = reinterpret_cast<char*>(buffer);
+  while (size_dword > 0) {
+    DWORD bytes_read;
+    BOOL success = ::ReadFile(file, buffer_c, size_dword, &bytes_read, nullptr);

[Mark Mentovai, 2014/12/17 05:39:16]

If EOF is supposed to be signaled by ReadFile() returning true and setting
bytes_read to 0, then this loop won’t ever terminate when encountering EOF
before the buffer is filled. It seems like that could happen when trying to read
a 0-byte or 1-byte file with a 4096-byte buffer. Maybe it can also happen when
reading from a named pipe and the writer closes it? The doc says that you get
ERROR_BROKEN_PIPE when the writer closes an anonymous pipe, but it doesn’t call
out named pipes for this. And it looks like ERROR_HANDLE_EOF only happens with
an asynchronous read.

I don’t know if that can be distinguished from the case you mentioned from the
doc, reading from a pipe that had a 0-byte write done to it.

[scottmg, 2014/12/17 20:18:32]

Yes, you're right. (I confirmed this, which I should have done before.) So a 0
byte read success on a file will break now.
I can't find documentation that explicitly says that, but the example here for a
named pipe does
http://msdn.microsoft.com/en-ca/library/windows/desktop/aa365588(v=vs.85).aspx
handle ERROR_BROKEN_PIPE. Also, from
http://msdn.microsoft.com/en-ca/library/windows/desktop/aa365141(v=vs.85).aspx

"""
Anonymous pipes are implemented using a named pipe with a unique name.
Therefore, you can often pass a handle to an anonymous pipe to a function that
requires a handle to a named pipe.
"""

So I'm fairly confident it will cause a legitimate failure.
It doesn't seem like it based on just the return values. Making 0 bytes == EOF
only when GetFileType(file) == FILE_TYPE_PIPE seems close, but the documentation
for that http://msdn.microsoft.com/en-us/library/aa364960%28VS.85%29.aspx
includes "socket" in the list of things that are FILE_TYPE_PIPE. Looking at the
WinSock docs, I'm starting to think that maybe a fully generic Read isn't a
great idea on Windows!
http://msdn.microsoft.com/en-us/library/windows/desktop/ms740522(v=vs.85).aspx .
I'm also not sure what 0 bytes read successfully from a socket should mean.

Another option would be to call GetNamedPipeInfo
http://msdn.microsoft.com/en-ca/library/windows/desktop/aa365445%28v=vs.85%29...
which should fail for non-pipes.

Given that the socket docs suggest that it is a poor idea to use
ReadFile/WriteFile on sockets in preference for WSARecv/WSASend, and that
passing a SOCKET to a function requiring a HANDLE requires a reinterpret_cast,
I've added the GetFileType(file) == FILE_TYPE_PIPE to the EOF case, as well as a
DCHECK that the handle in question is not a socket based on the above checks.

+    if (!success && GetLastError() != ERROR_MORE_DATA)
+      return -1;
+
+    buffer_c += bytes_read;
+    size_dword -= bytes_read;
+    total_read += bytes_read;
   }
+  return total_read;
+}
 
-  LOG(WARNING) << "value " << source << " out of range";
-  return static_cast<Destination>(default_value);
+ssize_t WriteFile(FileHandle file, const void* buffer, size_t size) {
+  // TODO(scottmg): This might need to handle the limit for pipes across a
+  // network in the future.
+  DWORD size_dword = base::checked_cast<DWORD>(size);
+  DWORD bytes_written;
+  BOOL rv = ::WriteFile(file, buffer, size_dword, &bytes_written, nullptr);
+  if (!rv)
+    return -1;
+  CHECK_EQ(bytes_written, size_dword);
+  return bytes_written;
 }
 
-}  // namespace crashpad
+bool LoggingCloseFile(FileHandle file) {
+  BOOL rv = CloseHandle(file);
+  PLOG_IF(ERROR, !rv) << "CloseHandle";
+  return rv;
+}
 
-#endif  // CRASHPAD_UTIL_NUMERIC_IN_RANGE_CAST_H_
+}  // namespace crashpad