From c1a6b128c710b2cde0bad5033052aa737291635e Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Mon, 3 Jul 2017 09:25:55 +0000 Subject: [PATCH] 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 | 18 ++--- lldb/source/Host/common/File.cpp | 74 +++++++------------ lldb/source/Host/macosx/Host.mm | 6 +- .../posix/ConnectionFileDescriptorPosix.cpp | 21 ++---- .../Process/FreeBSD/ProcessMonitor.cpp | 24 ++---- .../Process/Linux/NativeProcessLinux.cpp | 19 ++--- .../Process/NetBSD/NativeProcessNetBSD.cpp | 7 +- 7 files changed, 60 insertions(+), 109 deletions(-) diff --git a/lldb/include/lldb/Host/Host.h b/lldb/include/lldb/Host/Host.h index c41e4796f532..da0b8e14c4a7 100644 --- a/lldb/include/lldb/Host/Host.h +++ b/lldb/include/lldb/Host/Host.h @@ -7,14 +7,8 @@ // //===----------------------------------------------------------------------===// -#ifndef liblldb_Host_h_ -#define liblldb_Host_h_ -#if defined(__cplusplus) - -#include - -#include -#include +#ifndef LLDB_HOST_HOST_H +#define LLDB_HOST_HOST_H #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 +#include +#include +#include +#include namespace lldb_private { @@ -254,5 +253,4 @@ template <> struct format_provider { }; } // namespace llvm -#endif // #if defined(__cplusplus) -#endif // liblldb_Host_h_ +#endif // LLDB_HOST_HOST_H diff --git a/lldb/source/Host/common/File.cpp b/lldb/source/Host/common/File.cpp index 3de93ebc220b..38cbb78f1509 100644 --- a/lldb/source/Host/common/File.cpp +++ b/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 = ::fdopen(m_descriptor, mode); - } while (m_stream == NULL && errno == EINTR); + m_stream = + llvm::sys::RetryAfterSignal(nullptr, ::fdopen, m_descriptor, mode); // 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 { -#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); - + m_descriptor = llvm::sys::RetryAfterSignal(-1, DoOpen, path, oflag, mode); 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; - do { - err = ::fflush(m_stream); - } while (err == EOF && errno == EINTR); - - if (err == EOF) + if (llvm::sys::RetryAfterSignal(EOF, ::fflush, m_stream) == 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; - do { - err = ::fsync(m_descriptor); - } while (err == -1 && errno == EINTR); - - if (err == -1) + if (llvm::sys::RetryAfterSignal(-1, ::fsync, m_descriptor) == -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 = ::read(m_descriptor, buf, num_bytes); - } while (bytes_read < 0 && errno == EINTR); - + bytes_read = llvm::sys::RetryAfterSignal(-1, ::read, m_descriptor, buf, num_bytes); 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 = ::write(m_descriptor, buf, num_bytes); - } while (bytes_written < 0 && errno == EINTR); - + bytes_written = + llvm::sys::RetryAfterSignal(-1, ::write, m_descriptor, buf, num_bytes); 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; - do { - bytes_read = ::pread(fd, buf, num_bytes, offset); - } while (bytes_read < 0 && errno == EINTR); - + ssize_t bytes_read = + llvm::sys::RetryAfterSignal(-1, ::pread, fd, buf, num_bytes, offset); 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; - do { - bytes_written = ::pwrite(m_descriptor, buf, num_bytes, offset); - } while (bytes_written < 0 && errno == EINTR); - + ssize_t bytes_written = + llvm::sys::RetryAfterSignal(-1, ::pwrite, m_descriptor, buf, num_bytes, offset); if (bytes_written < 0) { num_bytes = 0; error.SetErrorToErrno(); diff --git a/lldb/source/Host/macosx/Host.mm b/lldb/source/Host/macosx/Host.mm index bbf70cd4c4b3..75624ef21f43 100644 --- a/lldb/source/Host/macosx/Host.mm +++ b/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 = ::waitpid(pid, &status, 0); - } while (wait_pid < 0 && errno == EINTR); - + wait_pid = llvm::sys::RetryAfterSignal(-1, ::waitpid, pid, &status, 0); if (wait_pid >= 0) { int signal = 0; int exit_status = 0; diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp index 6b0f069c35a9..105ef0f23d46 100644 --- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp +++ b/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; - do { - fd = ::open(addr_str.c_str(), O_RDWR); - } while (fd == -1 && errno == EINTR); - + int fd = llvm::sys::RetryAfterSignal(-1, ::open, addr_str.c_str(), O_RDWR); if (fd == -1) { if (error_ptr) error_ptr->SetErrorToErrno(); @@ -620,20 +616,17 @@ ConnectionFileDescriptor::BytesAvailable(const Timeout &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; - - do { - bytes_read = ::read(pipe_fd, buffer, sizeof(buffer)); - } while (bytes_read < 0 && errno == EINTR); - - switch (buffer[0]) { + ssize_t bytes_read = llvm::sys::RetryAfterSignal(-1, ::read, pipe_fd, &c, 1); + assert(bytes_read == 1); + (void)bytes_read; + switch (c) { case 'q': if (log) log->Printf("%p ConnectionFileDescriptor::BytesAvailable() " "got data: %c from the command channel.", - static_cast(this), buffer[0]); + static_cast(this), c); return eConnectionStatusEndOfFile; case 'i': // Interrupt the current read diff --git a/lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp b/lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp index 10dd14753914..a4f5f02dde62 100644 --- a/lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp +++ b/lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp @@ -746,15 +746,9 @@ ProcessMonitor::ProcessMonitor( if (!error.Success()) return; -WAIT_AGAIN: - // Wait for the operation thread to initialize. - if (sem_wait(&args->m_semaphore)) { - if (errno == EINTR) - goto WAIT_AGAIN; - else { - error.SetErrorToErrno(); - return; - } + if (llvm::sys::RetryAfterSignal(-1, sem_wait, &args->m_semaphore) == -1) { + 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: - // Wait for the operation thread to initialize. - if (sem_wait(&args->m_semaphore)) { - if (errno == EINTR) - goto WAIT_AGAIN; - else { - error.SetErrorToErrno(); - return; - } + if (llvm::sys::RetryAfterSignal(-1, sem_wait, &args->m_semaphore) == -1) { + error.SetErrorToErrno(); + return; } // Check that the attach was a success. diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp index 8e378802de9c..713d56d2bf59 100644 --- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp +++ b/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; - do { - LLDB_LOG(log, - "received thread creation event for tid {0}. tid not tracked " - "yet, waiting for thread to appear...", - tid); - wait_pid = waitpid(tid, &status, __WALL); - } while (wait_pid == -1 && errno == EINTR); + LLDB_LOG(log, + "received thread creation event for tid {0}. tid not tracked " + "yet, waiting for thread to appear...", + tid); + ::pid_t wait_pid = llvm::sys::RetryAfterSignal(-1, ::waitpid, tid, &status, __WALL); // 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; diff --git a/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp b/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp index a4d775860a65..40193342fc7f 100644 --- a/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp +++ b/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); }