Cross platform low level file IO wrappers

util/file/file_io_win.cc

 // Copyright 2014 The Crashpad Authors. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
 //
 //     http://www.apache.org/licenses/LICENSE-2.0
 //
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // 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