Use llvm::sys::RetryAfterSignal instead of a manual while errno!=EINTR loop

Reviewers: zturner, eugene, krytarowski

Subscribers: emaste, mgorny, lldb-commits

Differential Revision: https://reviews.llvm.org/D33831

llvm-svn: 307009

lldb/include/lldb/Host/Host.h

@@ -7,14 +7,8 @@
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef liblldb_Host_h_
+#ifndef LLDB_HOST_HOST_H
-#define liblldb_Host_h_
+#define LLDB_HOST_HOST_H
-#if defined(__cplusplus)
-
-#include <stdarg.h>
-
-#include <map>
-#include <string>
 
 #include "lldb/Host/File.h"
 #include "lldb/Host/HostThread.h"
@@ -22,6 +16,11 @@
 #include "lldb/Utility/StringList.h"
 #include "lldb/lldb-private-forward.h"
 #include "lldb/lldb-private.h"
+#include <cerrno>
+#include <map>
+#include <stdarg.h>
+#include <string>
+#include <type_traits>
 
 namespace lldb_private {
 
@@ -254,5 +253,4 @@ template <> struct format_provider<lldb_private::WaitStatus> {
 };
 } // namespace llvm
 
-#endif // #if defined(__cplusplus)
+#endif // LLDB_HOST_HOST_H
-#endif // liblldb_Host_h_

lldb/source/Host/common/File.cpp

@@ -24,10 +24,12 @@
 #endif
 
 #include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/Errno.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Process.h" // for llvm::sys::Process::FileDescriptorHasColors()
 
 #include "lldb/Host/Config.h"
+#include "lldb/Host/Host.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Log.h"
@@ -133,9 +135,8 @@ FILE *File::GetStream() {
           m_should_close_fd = true;
         }
 
-        do {
+        m_stream =
-          m_stream = ::fdopen(m_descriptor, mode);
+            llvm::sys::RetryAfterSignal(nullptr, ::fdopen, m_descriptor, mode);
-        } while (m_stream == NULL && errno == EINTR);
 
         // If we got a stream, then we own the stream and should no
         // longer own the descriptor because fclose() will close it for us
@@ -157,6 +158,19 @@ void File::SetStream(FILE *fh, bool transfer_ownership) {
   m_own_stream = transfer_ownership;
 }
 
+static int DoOpen(const char *path, int flags, int mode) {
+#ifdef _MSC_VER
+  std::wstring wpath;
+  if (!llvm::ConvertUTF8toWide(path, wpath))
+    return -1;
+  int result;
+  ::_wsopen_s(&result, wpath.c_str(), oflag, _SH_DENYNO, mode);
+  return result;
+#else
+  return ::open(path, flags, mode);
+#endif
+}
+
 Status File::Open(const char *path, uint32_t options, uint32_t permissions) {
   Status error;
   if (IsValid())
@@ -222,20 +236,7 @@ Status File::Open(const char *path, uint32_t options, uint32_t permissions) {
       mode |= S_IXOTH;
   }
 
-  do {
+  m_descriptor = llvm::sys::RetryAfterSignal(-1, DoOpen, path, oflag, mode);
-#ifdef _MSC_VER
-    std::wstring wpath;
-    if (!llvm::ConvertUTF8toWide(path, wpath)) {
-      m_descriptor = -1;
-      error.SetErrorString("Error converting path to UTF-16");
-      return error;
-    }
-    ::_wsopen_s(&m_descriptor, wpath.c_str(), oflag, _SH_DENYNO, mode);
-#else
-    m_descriptor = ::open(path, oflag, mode);
-#endif
-  } while (m_descriptor < 0 && errno == EINTR);
-
   if (!DescriptorIsValid())
     error.SetErrorToErrno();
   else {
@@ -421,12 +422,7 @@ off_t File::SeekFromEnd(off_t offset, Status *error_ptr) {
 Status File::Flush() {
   Status error;
   if (StreamIsValid()) {
-    int err = 0;
+    if (llvm::sys::RetryAfterSignal(EOF, ::fflush, m_stream) == EOF)
-    do {
-      err = ::fflush(m_stream);
-    } while (err == EOF && errno == EINTR);
-
-    if (err == EOF)
       error.SetErrorToErrno();
   } else if (!DescriptorIsValid()) {
     error.SetErrorString("invalid file handle");
@@ -442,12 +438,7 @@ Status File::Sync() {
     if (err == 0)
       error.SetErrorToGenericError();
 #else
-    int err = 0;
+    if (llvm::sys::RetryAfterSignal(-1, ::fsync, m_descriptor) == -1)
-    do {
-      err = ::fsync(m_descriptor);
-    } while (err == -1 && errno == EINTR);
-
-    if (err == -1)
       error.SetErrorToErrno();
 #endif
   } else {
@@ -497,10 +488,7 @@ Status File::Read(void *buf, size_t &num_bytes) {
 
   ssize_t bytes_read = -1;
   if (DescriptorIsValid()) {
-    do {
+    bytes_read = llvm::sys::RetryAfterSignal(-1, ::read, m_descriptor, buf, num_bytes);
-      bytes_read = ::read(m_descriptor, buf, num_bytes);
-    } while (bytes_read < 0 && errno == EINTR);
-
     if (bytes_read == -1) {
       error.SetErrorToErrno();
       num_bytes = 0;
@@ -559,10 +547,8 @@ Status File::Write(const void *buf, size_t &num_bytes) {
 
   ssize_t bytes_written = -1;
   if (DescriptorIsValid()) {
-    do {
+    bytes_written =
-      bytes_written = ::write(m_descriptor, buf, num_bytes);
+        llvm::sys::RetryAfterSignal(-1, ::write, m_descriptor, buf, num_bytes);
-    } while (bytes_written < 0 && errno == EINTR);
-
     if (bytes_written == -1) {
       error.SetErrorToErrno();
       num_bytes = 0;
@@ -624,11 +610,8 @@ Status File::Read(void *buf, size_t &num_bytes, off_t &offset) {
 #ifndef _WIN32
   int fd = GetDescriptor();
   if (fd != kInvalidDescriptor) {
-    ssize_t bytes_read = -1;
+    ssize_t bytes_read =
-    do {
+        llvm::sys::RetryAfterSignal(-1, ::pread, fd, buf, num_bytes, offset);
-      bytes_read = ::pread(fd, buf, num_bytes, offset);
-    } while (bytes_read < 0 && errno == EINTR);
-
     if (bytes_read < 0) {
       num_bytes = 0;
       error.SetErrorToErrno();
@@ -730,11 +713,8 @@ Status File::Write(const void *buf, size_t &num_bytes, off_t &offset) {
   int fd = GetDescriptor();
   if (fd != kInvalidDescriptor) {
 #ifndef _WIN32
-    ssize_t bytes_written = -1;
+    ssize_t bytes_written =
-    do {
+        llvm::sys::RetryAfterSignal(-1, ::pwrite, m_descriptor, buf, num_bytes, offset);
-      bytes_written = ::pwrite(m_descriptor, buf, num_bytes, offset);
-    } while (bytes_written < 0 && errno == EINTR);
-
     if (bytes_written < 0) {
       num_bytes = 0;
       error.SetErrorToErrno();

lldb/source/Host/macosx/Host.mm

@@ -74,6 +74,7 @@
 #include "lldb/Utility/StructuredData.h"
 
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Errno.h"
 
 #include "cfcpp/CFCBundle.h"
 #include "cfcpp/CFCMutableArray.h"
@@ -1663,10 +1664,7 @@ HostThread Host::StartMonitoringChildProcess(
       int wait_pid = 0;
       bool cancel = false;
       bool exited = false;
-      do {
+      wait_pid = llvm::sys::RetryAfterSignal(-1, ::waitpid, pid, &status, 0);
-        wait_pid = ::waitpid(pid, &status, 0);
-      } while (wait_pid < 0 && errno == EINTR);
-
       if (wait_pid >= 0) {
         int signal = 0;
         int exit_status = 0;

lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp

@@ -245,11 +245,7 @@ ConnectionStatus ConnectionFileDescriptor::Connect(llvm::StringRef path,
     } else if ((addr = GetURLAddress(path, FILE_SCHEME))) {
       std::string addr_str = addr->str();
       // file:///PATH
-      int fd = -1;
+      int fd = llvm::sys::RetryAfterSignal(-1, ::open, addr_str.c_str(), O_RDWR);
-      do {
-        fd = ::open(addr_str.c_str(), O_RDWR);
-      } while (fd == -1 && errno == EINTR);
-
       if (fd == -1) {
         if (error_ptr)
           error_ptr->SetErrorToErrno();
@@ -620,20 +616,17 @@ ConnectionFileDescriptor::BytesAvailable(const Timeout<std::micro> &timeout,
         if (select_helper.FDIsSetRead(pipe_fd)) {
           // There is an interrupt or exit command in the command pipe
           // Read the data from that pipe:
-          char buffer[1];
+          char c;
 
-          ssize_t bytes_read;
+          ssize_t bytes_read = llvm::sys::RetryAfterSignal(-1, ::read, pipe_fd, &c, 1);
-
+          assert(bytes_read == 1);
-          do {
+          (void)bytes_read;
-            bytes_read = ::read(pipe_fd, buffer, sizeof(buffer));
+          switch (c) {
-          } while (bytes_read < 0 && errno == EINTR);
-
-          switch (buffer[0]) {
           case 'q':
             if (log)
               log->Printf("%p ConnectionFileDescriptor::BytesAvailable() "
                           "got data: %c from the command channel.",
-                          static_cast<void *>(this), buffer[0]);
+                          static_cast<void *>(this), c);
             return eConnectionStatusEndOfFile;
           case 'i':
             // Interrupt the current read

lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp

@@ -746,15 +746,9 @@ ProcessMonitor::ProcessMonitor(
   if (!error.Success())
     return;
 
-WAIT_AGAIN:
+  if (llvm::sys::RetryAfterSignal(-1, sem_wait, &args->m_semaphore) == -1) {
-  // Wait for the operation thread to initialize.
+    error.SetErrorToErrno();
-  if (sem_wait(&args->m_semaphore)) {
+    return;
-    if (errno == EINTR)
-      goto WAIT_AGAIN;
-    else {
-      error.SetErrorToErrno();
-      return;
-    }
   }
 
   // Check that the launch was a success.
@@ -790,15 +784,9 @@ ProcessMonitor::ProcessMonitor(ProcessFreeBSD *process, lldb::pid_t pid,
   if (!error.Success())
     return;
 
-WAIT_AGAIN:
+  if (llvm::sys::RetryAfterSignal(-1, sem_wait, &args->m_semaphore) == -1) {
-  // Wait for the operation thread to initialize.
+    error.SetErrorToErrno();
-  if (sem_wait(&args->m_semaphore)) {
+    return;
-    if (errno == EINTR)
-      goto WAIT_AGAIN;
-    else {
-      error.SetErrorToErrno();
-      return;
-    }
   }
 
   // Check that the attach was a success.

lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp

@@ -662,14 +662,11 @@ void NativeProcessLinux::WaitForNewThread(::pid_t tid) {
 
   // The thread is not tracked yet, let's wait for it to appear.
   int status = -1;
-  ::pid_t wait_pid;
+  LLDB_LOG(log,
-  do {
+           "received thread creation event for tid {0}. tid not tracked "
-    LLDB_LOG(log,
+           "yet, waiting for thread to appear...",
-             "received thread creation event for tid {0}. tid not tracked "
+           tid);
-             "yet, waiting for thread to appear...",
+  ::pid_t wait_pid = llvm::sys::RetryAfterSignal(-1, ::waitpid, tid, &status, __WALL);
-             tid);
-    wait_pid = waitpid(tid, &status, __WALL);
-  } while (wait_pid == -1 && errno == EINTR);
   // Since we are waiting on a specific tid, this must be the creation event.
   // But let's do some checks just in case.
   if (wait_pid != tid) {
@@ -2363,15 +2360,13 @@ void NativeProcessLinux::SigchldHandler() {
   // Process all pending waitpid notifications.
   while (true) {
     int status = -1;
-    ::pid_t wait_pid = waitpid(-1, &status, __WALL | __WNOTHREAD | WNOHANG);
+    ::pid_t wait_pid = llvm::sys::RetryAfterSignal(-1, ::waitpid, -1, &status,
+                                          __WALL | __WNOTHREAD | WNOHANG);
 
     if (wait_pid == 0)
       break; // We are done.
 
     if (wait_pid == -1) {
-      if (errno == EINTR)
-        continue;
-
       Status error(errno, eErrorTypePOSIX);
       LLDB_LOG(log, "waitpid (-1, &status, _) failed: {0}", error);
       break;

lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp

@@ -21,6 +21,7 @@
 #include "lldb/Host/common/NativeRegisterContext.h"
 #include "lldb/Host/posix/ProcessLauncherPosixFork.h"
 #include "lldb/Target/Process.h"
+#include "llvm/Support/Errno.h"
 
 // System includes - They have to be included after framework includes because
 // they define some
@@ -820,15 +821,13 @@ void NativeProcessNetBSD::SigchldHandler() {
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
   // Process all pending waitpid notifications.
   int status;
-  ::pid_t wait_pid = waitpid(GetID(), &status, WALLSIG | WNOHANG);
+  ::pid_t wait_pid =
+      llvm::sys::RetryAfterSignal(-1, waitpid, GetID(), &status, WALLSIG | WNOHANG);
 
   if (wait_pid == 0)
     return; // We are done.
 
   if (wait_pid == -1) {
-    if (errno == EINTR)
-      return;
-
     Status error(errno, eErrorTypePOSIX);
     LLDB_LOG(log, "waitpid ({0}, &status, _) failed: {1}", GetID(), error);
   }