Cross platform low level file IO wrappers

util/file/file_io_win.cc

Index: util/file/file_io_win.cc
diff --git a/util/synchronization/semaphore.cc b/util/file/file_io_win.cc
similarity index 44%
copy from util/synchronization/semaphore.cc
copy to util/file/file_io_win.cc
index d513fa0dd43981acc72b1ff155fa2d1e77616388..e6e865d1460b9911c00088d7090371d0fa514cbb 100644
--- a/util/synchronization/semaphore.cc
+++ b/util/file/file_io_win.cc
@@ -12,50 +12,39 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "util/synchronization/semaphore.h"
+#include "util/file/file_io.h"
 
 #include "base/logging.h"
-#include "base/posix/eintr_wrapper.h"
+#include "base/numerics/safe_conversions.h"
 
 namespace crashpad {
 
-#if defined(OS_MACOSX)
+// TODO(scottmg): Handle > DWORD sized writes if necessary.
 
-Semaphore::Semaphore(int value)
-    : semaphore_(dispatch_semaphore_create(value)) {
-  CHECK(semaphore_) << "dispatch_semaphore_create";
+ssize_t ReadFile(HANDLE file, void* buffer, size_t size) {

[Mark Mentovai, 2014/12/16 22:40:25]

FileHandle, not HANDLE. Same on lines 33 and 44.

[scottmg, 2014/12/17 00:22:58]

Done.

+  DWORD size_dword = base::checked_cast<DWORD>(size);
+  DWORD bytes_read;
+  BOOL rv = ::ReadFile(file, buffer, size_dword, &bytes_read, NULL);

[Mark Mentovai, 2014/12/16 22:40:25]

Crashpad is 100% nullptr now. Same on line 38.

[Mark Mentovai, 2014/12/16 22:40:25]

Do ReadFile() and WriteFile() guarantee not to do a short read or write when
doing blocking I/O and not at EOF? Even on a network stream or a networked file?

Based on the docs, I think that short reads and writes are possible at least on
pipes. You left a TODO for something similar in WriteFile(), but I think that
ReadFile() might be affected too. It at least deserves a comment, even if you
aren’t ready to implement it. (And in that case, the functions would probably
become involved enough that the traitsy thing that the POSIX implementation does
would be advantageous.)

[scottmg, 2014/12/17 00:22:58]

Done.

[scottmg, 2014/12/17 00:22:58]

I think you're right. From my reading of the docs, I believe ReadFile can read
short when reading from a pipe, but WriteFile simply blocks when used in
synchronous mode. Because of this difference, I didn't switch to traits.

The comment in WriteFile is still there and refers to an unrelated limit when
writing to a networked pipe on XP -- writes must be chunked to < 32M or else
they'll just fail.

+  if (!rv)
+    return -1;
+  return bytes_read;
 }
 
-Semaphore::~Semaphore() {
-  dispatch_release(semaphore_);
+ssize_t WriteFile(HANDLE 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, NULL);
+  if (!rv)
+    return -1;
+  return bytes_written;
 }
 
-void Semaphore::Wait() {
-  CHECK_EQ(dispatch_semaphore_wait(semaphore_, DISPATCH_TIME_FOREVER), 0);
+bool LoggingCloseFile(HANDLE file) {
+  BOOL rv = ::CloseHandle(file);

[Mark Mentovai, 2014/12/16 22:40:25]

The :: aren’t necessary on this one.

[scottmg, 2014/12/17 00:22:58]

Done.

Habit to imply "this is a Win32 function", applied inconsistently in Chromium.
Either way is fine with me.

+  PLOG_IF(ERROR, !rv) << "CloseHandle";
+  return rv;
 }
 
-void Semaphore::Signal() {
-  dispatch_semaphore_signal(semaphore_);
-}
-
-#else
-
-Semaphore::Semaphore(int value) {
-  PCHECK(sem_init(&semaphore_, 0, value) == 0) << "sem_init";
-}
-
-Semaphore::~Semaphore() {
-  PCHECK(sem_destroy(&semaphore_)) << "sem_destroy";
-}
-
-void Semaphore::Wait() {
-  PCHECK(HANDLE_EINTR(sem_wait(&semaphore_))) << "sem_wait";
-}
-
-void Semaphore::Signal() {
-  PCHECK(sem_post(&semaphore_)) << "sem_post";
-}
-
-#endif
-
 }  // namespace crashpad