[Lldb-commits] [lldb] e4cc6e9 - [lldb] Modernize PseudoTerminal::Fork
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Mon Oct 26 04:01:31 PDT 2020
Author: Pavel Labath
Date: 2020-10-26T12:01:20+01:00
New Revision: e4cc6e9bcdff5fe979ab72025cb803d723cd9c31
URL: https://github.com/llvm/llvm-project/commit/e4cc6e9bcdff5fe979ab72025cb803d723cd9c31
DIFF: https://github.com/llvm/llvm-project/commit/e4cc6e9bcdff5fe979ab72025cb803d723cd9c31.diff
LOG: [lldb] Modernize PseudoTerminal::Fork
Added:
Modified:
lldb/include/lldb/Host/PseudoTerminal.h
lldb/source/Host/common/PseudoTerminal.cpp
lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Host/PseudoTerminal.h b/lldb/include/lldb/Host/PseudoTerminal.h
index f7f8e636730e..350f926dcac1 100644
--- a/lldb/include/lldb/Host/PseudoTerminal.h
+++ b/lldb/include/lldb/Host/PseudoTerminal.h
@@ -62,15 +62,11 @@ class PseudoTerminal {
/// @li PseudoTerminal::ReleasePrimaryFileDescriptor() @li
/// PseudoTerminal::ReleaseSaveFileDescriptor()
///
- /// \param[out] error_str
- /// An pointer to an error that can describe any errors that
- /// occur. This can be NULL if no error status is desired.
- ///
/// \return
/// \b Parent process: a child process ID that is greater
- /// than zero, or -1 if the fork fails.
+ /// than zero, or an error if the fork fails.
/// \b Child process: zero.
- lldb::pid_t Fork(char *error_str, size_t error_len);
+ llvm::Expected<lldb::pid_t> Fork();
/// The primary file descriptor accessor.
///
diff --git a/lldb/source/Host/common/PseudoTerminal.cpp b/lldb/source/Host/common/PseudoTerminal.cpp
index dc0298cee849..c5f101caffe3 100644
--- a/lldb/source/Host/common/PseudoTerminal.cpp
+++ b/lldb/source/Host/common/PseudoTerminal.cpp
@@ -28,12 +28,6 @@ int posix_openpt(int flags);
using namespace lldb_private;
-// Write string describing error number
-static void ErrnoToStr(char *error_str, size_t error_len) {
- std::string strerror = llvm::sys::StrError();
- ::snprintf(error_str, error_len, "%s", strerror.c_str());
-}
-
// PseudoTerminal constructor
PseudoTerminal::PseudoTerminal()
: m_primary_fd(invalid_fd), m_secondary_fd(invalid_fd) {}
@@ -123,79 +117,47 @@ std::string PseudoTerminal::GetSecondaryName() const {
#endif
}
-// Fork a child process and have its stdio routed to a pseudo terminal.
-//
-// In the parent process when a valid pid is returned, the primary file
-// descriptor can be used as a read/write access to stdio of the child process.
-//
-// In the child process the stdin/stdout/stderr will already be routed to the
-// secondary pseudo terminal and the primary file descriptor will be closed as
-// it is no longer needed by the child process.
-//
-// This class will close the file descriptors for the primary/secondary when the
-// destructor is called, so be sure to call ReleasePrimaryFileDescriptor() or
-// ReleaseSecondaryFileDescriptor() if any file descriptors are going to be used
-// past the lifespan of this object.
-//
-// RETURNS:
-// in the parent process: the pid of the child, or -1 if fork fails
-// in the child process: zero
-lldb::pid_t PseudoTerminal::Fork(char *error_str, size_t error_len) {
- if (error_str)
- error_str[0] = '\0';
- pid_t pid = LLDB_INVALID_PROCESS_ID;
+llvm::Expected<lldb::pid_t> PseudoTerminal::Fork() {
#if LLDB_ENABLE_POSIX
- if (llvm::Error Err = OpenFirstAvailablePrimary(O_RDWR | O_CLOEXEC)) {
- snprintf(error_str, error_len, "%s", toString(std::move(Err)).c_str());
- return LLDB_INVALID_PROCESS_ID;
- }
+ if (llvm::Error Err = OpenFirstAvailablePrimary(O_RDWR | O_CLOEXEC))
+ return std::move(Err);
- pid = ::fork();
+ pid_t pid = ::fork();
if (pid < 0) {
- // Fork failed
- if (error_str)
- ErrnoToStr(error_str, error_len);
- } else if (pid == 0) {
- // Child Process
- ::setsid();
-
- if (llvm::Error Err = OpenSecondary(O_RDWR)) {
- snprintf(error_str, error_len, "%s", toString(std::move(Err)).c_str());
- return LLDB_INVALID_PROCESS_ID;
- }
+ return llvm::errorCodeToError(
+ std::error_code(errno, std::generic_category()));
+ }
+ if (pid > 0) {
+ // Parent process.
+ return pid;
+ }
- // Primary FD should have O_CLOEXEC set, but let's close it just in
- // case...
- ClosePrimaryFileDescriptor();
+ // Child Process
+ ::setsid();
-#if defined(TIOCSCTTY)
- // Acquire the controlling terminal
- if (::ioctl(m_secondary_fd, TIOCSCTTY, (char *)0) < 0) {
- if (error_str)
- ErrnoToStr(error_str, error_len);
- }
-#endif
- // Duplicate all stdio file descriptors to the secondary pseudo terminal
- if (::dup2(m_secondary_fd, STDIN_FILENO) != STDIN_FILENO) {
- if (error_str && !error_str[0])
- ErrnoToStr(error_str, error_len);
- }
+ if (llvm::Error Err = OpenSecondary(O_RDWR))
+ return std::move(Err);
- if (::dup2(m_secondary_fd, STDOUT_FILENO) != STDOUT_FILENO) {
- if (error_str && !error_str[0])
- ErrnoToStr(error_str, error_len);
- }
+ // Primary FD should have O_CLOEXEC set, but let's close it just in
+ // case...
+ ClosePrimaryFileDescriptor();
- if (::dup2(m_secondary_fd, STDERR_FILENO) != STDERR_FILENO) {
- if (error_str && !error_str[0])
- ErrnoToStr(error_str, error_len);
+#if defined(TIOCSCTTY)
+ // Acquire the controlling terminal
+ if (::ioctl(m_secondary_fd, TIOCSCTTY, (char *)0) < 0) {
+ return llvm::errorCodeToError(
+ std::error_code(errno, std::generic_category()));
+ }
+#endif
+ // Duplicate all stdio file descriptors to the secondary pseudo terminal
+ for (int fd : {STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO}) {
+ if (::dup2(m_secondary_fd, fd) != fd) {
+ return llvm::errorCodeToError(
+ std::error_code(errno, std::generic_category()));
}
- } else {
- // Parent Process
- // Do nothing and let the pid get returned!
}
#endif
- return pid;
+ return 0;
}
// The primary file descriptor accessor. This object retains ownership of the
diff --git a/lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp b/lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
index 6a9209d1214c..e85470d81e2f 100644
--- a/lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
+++ b/lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
@@ -821,17 +821,14 @@ bool ProcessMonitor::Launch(LaunchArgs *args) {
const FileSpec &working_dir = args->m_working_dir;
PseudoTerminal terminal;
- const size_t err_len = 1024;
- char err_str[err_len];
- ::pid_t pid;
// Propagate the environment if one is not supplied.
Environment::Envp envp =
(args->m_env.empty() ? Host::GetEnvironment() : args->m_env).getEnvp();
- if ((pid = terminal.Fork(err_str, err_len)) == -1) {
- args->m_error.SetErrorToGenericError();
- args->m_error.SetErrorString("Process fork failed.");
+ Expected<lldb::pid_t> pid = terminal.Fork();
+ if (!pid) {
+ args->m_error = pid.takeError();
goto FINISH;
}
@@ -847,7 +844,7 @@ bool ProcessMonitor::Launch(LaunchArgs *args) {
};
// Child process.
- if (pid == 0) {
+ if (*pid == 0) {
// Trace this process.
if (PTRACE(PT_TRACE_ME, 0, NULL, 0) < 0)
exit(ePtraceFailed);
@@ -892,7 +889,7 @@ bool ProcessMonitor::Launch(LaunchArgs *args) {
// Wait for the child process to to trap on its call to execve.
::pid_t wpid;
int status;
- if ((wpid = waitpid(pid, &status, 0)) < 0) {
+ if ((wpid = waitpid(*pid, &status, 0)) < 0) {
args->m_error.SetErrorToErrno();
goto FINISH;
} else if (WIFEXITED(status)) {
@@ -926,13 +923,13 @@ bool ProcessMonitor::Launch(LaunchArgs *args) {
}
goto FINISH;
}
- assert(WIFSTOPPED(status) && wpid == (::pid_t)pid &&
+ assert(WIFSTOPPED(status) && wpid == (::pid_t)*pid &&
"Could not sync with inferior process.");
#ifdef notyet
// Have the child raise an event on exit. This is used to keep the child in
// limbo until it is destroyed.
- if (PTRACE(PTRACE_SETOPTIONS, pid, NULL, PTRACE_O_TRACEEXIT) < 0) {
+ if (PTRACE(PTRACE_SETOPTIONS, *pid, NULL, PTRACE_O_TRACEEXIT) < 0) {
args->m_error.SetErrorToErrno();
goto FINISH;
}
@@ -940,7 +937,7 @@ bool ProcessMonitor::Launch(LaunchArgs *args) {
// Release the master terminal descriptor and pass it off to the
// ProcessMonitor instance. Similarly stash the inferior pid.
monitor->m_terminal_fd = terminal.ReleasePrimaryFileDescriptor();
- monitor->m_pid = pid;
+ monitor->m_pid = *pid;
// Set the terminal fd to be in non blocking mode (it simplifies the
// implementation of ProcessFreeBSD::GetSTDOUT to have a non-blocking
@@ -948,7 +945,7 @@ bool ProcessMonitor::Launch(LaunchArgs *args) {
if (!EnsureFDFlags(monitor->m_terminal_fd, O_NONBLOCK, args->m_error))
goto FINISH;
- process.SendMessage(ProcessMessage::Attach(pid));
+ process.SendMessage(ProcessMessage::Attach(*pid));
FINISH:
return args->m_error.Success();
More information about the lldb-commits
mailing list