labath updated this revision to Diff 101212.
labath added a comment.
- fix freebsd typo
- use ::waitpid consistently
https://reviews.llvm.org/D33831
Files:
include/lldb/Host/Host.h
source/Host/common/File.cpp
source/Host/macosx/Host.mm
source/Host/posix/ConnectionFileDescriptorPosix.cpp
source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
source/Plugins/Process/Linux/NativeProcessLinux.cpp
source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
unittests/Host/CMakeLists.txt
unittests/Host/HostTest.cpp
Index: unittests/Host/HostTest.cpp
===================================================================
--- /dev/null
+++ unittests/Host/HostTest.cpp
@@ -0,0 +1,33 @@
+//===-- HostTest.cpp --------------------------------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Host/Host.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+TEST(Host, temp_failure_retry) {
+ EXPECT_EQ(1, temp_failure_retry(-1, [] { return 1; }));
+
+ EXPECT_EQ(-1, temp_failure_retry(-1, [] {
+ errno = EAGAIN;
+ return -1;
+ }));
+ EXPECT_EQ(EAGAIN, errno);
+
+ unsigned calls = 0;
+ EXPECT_EQ(1, temp_failure_retry(-1, [&calls] {
+ errno = EINTR;
+ ++calls;
+ return calls == 1 ? -1 : 1;
+ }));
+ EXPECT_EQ(2u, calls);
+
+ EXPECT_EQ(1, temp_failure_retry(-1, [](int x) { return x; }, 1));
+}
Index: unittests/Host/CMakeLists.txt
===================================================================
--- unittests/Host/CMakeLists.txt
+++ unittests/Host/CMakeLists.txt
@@ -1,6 +1,7 @@
set (FILES
FileSpecTest.cpp
FileSystemTest.cpp
+ HostTest.cpp
MainLoopTest.cpp
SocketAddressTest.cpp
SocketTest.cpp
Index: source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
===================================================================
--- source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
+++ source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
@@ -848,15 +848,13 @@
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 =
+ temp_failure_retry(-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);
}
Index: source/Plugins/Process/Linux/NativeProcessLinux.cpp
===================================================================
--- source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -688,14 +688,11 @@
// 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 = temp_failure_retry(-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) {
@@ -2370,15 +2367,13 @@
// Process all pending waitpid notifications.
while (true) {
int status = -1;
- ::pid_t wait_pid = waitpid(-1, &status, __WALL | __WNOTHREAD | WNOHANG);
+ ::pid_t wait_pid = temp_failure_retry(-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;
Index: source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
===================================================================
--- source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
+++ source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
@@ -747,15 +747,9 @@
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 (temp_failure_retry(-1, sem_wait, &args->m_semaphore) == -1) {
+ error.SetErrorToErrno();
+ return;
}
// Check that the launch was a success.
@@ -791,15 +785,9 @@
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 (temp_failure_retry(-1, sem_wait, &args->m_semaphore) == -1) {
+ error.SetErrorToErrno();
+ return;
}
// Check that the attach was a success.
Index: source/Host/posix/ConnectionFileDescriptorPosix.cpp
===================================================================
--- source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -245,11 +245,7 @@
} 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 = temp_failure_retry(-1, ::open, addr_str.c_str(), O_RDWR);
if (fd == -1) {
if (error_ptr)
error_ptr->SetErrorToErrno();
@@ -622,20 +618,17 @@
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];
-
- ssize_t bytes_read;
-
- do {
- bytes_read = ::read(pipe_fd, buffer, sizeof(buffer));
- } while (bytes_read < 0 && errno == EINTR);
+ char c;
- switch (buffer[0]) {
+ ssize_t bytes_read = temp_failure_retry(-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<void *>(this), buffer[0]);
+ static_cast<void *>(this), c);
return eConnectionStatusEndOfFile;
case 'i':
// Interrupt the current read
Index: source/Host/macosx/Host.mm
===================================================================
--- source/Host/macosx/Host.mm
+++ source/Host/macosx/Host.mm
@@ -1367,10 +1367,7 @@
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 = temp_failure_retry(-1, ::waitpid, pid, &status, 0);
if (wait_pid >= 0) {
int signal = 0;
int exit_status = 0;
Index: source/Host/common/File.cpp
===================================================================
--- source/Host/common/File.cpp
+++ source/Host/common/File.cpp
@@ -28,6 +28,7 @@
#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 +134,7 @@
m_should_close_fd = true;
}
- do {
- m_stream = ::fdopen(m_descriptor, mode);
- } while (m_stream == NULL && errno == EINTR);
+ m_stream = temp_failure_retry(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 +156,19 @@
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 +234,7 @@
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 = temp_failure_retry(-1, DoOpen, path, oflag, mode);
if (!DescriptorIsValid())
error.SetErrorToErrno();
else {
@@ -421,12 +420,7 @@
Status File::Flush() {
Status error;
if (StreamIsValid()) {
- int err = 0;
- do {
- err = ::fflush(m_stream);
- } while (err == EOF && errno == EINTR);
-
- if (err == EOF)
+ if (temp_failure_retry(EOF, ::fflush, m_stream) == EOF)
error.SetErrorToErrno();
} else if (!DescriptorIsValid()) {
error.SetErrorString("invalid file handle");
@@ -442,12 +436,7 @@
if (err == 0)
error.SetErrorToGenericError();
#else
- int err = 0;
- do {
- err = ::fsync(m_descriptor);
- } while (err == -1 && errno == EINTR);
-
- if (err == -1)
+ if (temp_failure_retry(-1, ::fsync, m_descriptor) == -1)
error.SetErrorToErrno();
#endif
} else {
@@ -497,10 +486,7 @@
ssize_t bytes_read = -1;
if (DescriptorIsValid()) {
- do {
- bytes_read = ::read(m_descriptor, buf, num_bytes);
- } while (bytes_read < 0 && errno == EINTR);
-
+ bytes_read = temp_failure_retry(-1, ::read, m_descriptor, buf, num_bytes);
if (bytes_read == -1) {
error.SetErrorToErrno();
num_bytes = 0;
@@ -559,10 +545,8 @@
ssize_t bytes_written = -1;
if (DescriptorIsValid()) {
- do {
- bytes_written = ::write(m_descriptor, buf, num_bytes);
- } while (bytes_written < 0 && errno == EINTR);
-
+ bytes_written =
+ temp_failure_retry(-1, ::write, m_descriptor, buf, num_bytes);
if (bytes_written == -1) {
error.SetErrorToErrno();
num_bytes = 0;
@@ -624,11 +608,8 @@
#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 =
+ temp_failure_retry(-1, ::pread, fd, buf, num_bytes, offset);
if (bytes_read < 0) {
num_bytes = 0;
error.SetErrorToErrno();
@@ -730,11 +711,8 @@
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 =
+ temp_failure_retry(-1, ::pwrite, m_descriptor, buf, num_bytes, offset);
if (bytes_written < 0) {
num_bytes = 0;
error.SetErrorToErrno();
Index: include/lldb/Host/Host.h
===================================================================
--- include/lldb/Host/Host.h
+++ include/lldb/Host/Host.h
@@ -7,21 +7,20 @@
//
//===----------------------------------------------------------------------===//
-#ifndef liblldb_Host_h_
-#define liblldb_Host_h_
-#if defined(__cplusplus)
-
-#include <stdarg.h>
-
-#include <map>
-#include <string>
+#ifndef LLDB_HOST_HOST_H
+#define LLDB_HOST_HOST_H
#include "lldb/Host/File.h"
#include "lldb/Host/HostThread.h"
#include "lldb/Utility/FileSpec.h"
#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 {
@@ -244,7 +243,16 @@
static size_t GetEnvironment(StringList &env);
};
+template <typename R, typename F, typename... Args>
+inline auto temp_failure_retry(const R &fail_value, const F &f,
+ const Args &... args) -> decltype(f(args...)) {
+ decltype(f(args...)) result;
+ do
+ result = f(args...);
+ while (result == fail_value && errno == EINTR);
+ return result;
+}
+
} // namespace lldb_private
-#endif // #if defined(__cplusplus)
-#endif // liblldb_Host_h_
+#endif // LLDB_HOST_HOST_H
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
