[lldb] Modernize PseudoTerminal::Fork

lldb/include/lldb/Host/PseudoTerminal.h

@@ -62,15 +62,11 @@ public:
   /// @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.
   ///

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_t pid = ::fork();
+  if (pid < 0) {
+    return llvm::errorCodeToError(
+        std::error_code(errno, std::generic_category()));
+  }
+  if (pid > 0) {
+    // Parent process.
+    return pid;
   }
 
-  pid = ::fork();
-  if (pid < 0) {
-    // Fork failed
-    if (error_str)
-      ErrnoToStr(error_str, error_len);
-  } else if (pid == 0) {
-    // Child Process
-    ::setsid();
+  // 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;
-    }
+  if (llvm::Error Err = OpenSecondary(O_RDWR))
+    return std::move(Err);
 
-    // Primary FD should have O_CLOEXEC set, but let's close it just in
-    // case...
-    ClosePrimaryFileDescriptor();
+  // Primary FD should have O_CLOEXEC set, but let's close it just in
+  // case...
+  ClosePrimaryFileDescriptor();
 
 #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 (::dup2(m_secondary_fd, STDOUT_FILENO) != STDOUT_FILENO) {
-      if (error_str && !error_str[0])
-        ErrnoToStr(error_str, error_len);
-    }
-
-    if (::dup2(m_secondary_fd, STDERR_FILENO) != STDERR_FILENO) {
-      if (error_str && !error_str[0])
-        ErrnoToStr(error_str, error_len);
-    }
-  } else {
-    // Parent Process
-    // Do nothing and let the pid get returned!
+  // Acquire the controlling terminal
+  if (::ioctl(m_secondary_fd, TIOCSCTTY, (char *)0) < 0) {
+    return llvm::errorCodeToError(
+        std::error_code(errno, std::generic_category()));
   }
 #endif
-  return pid;
+  // 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()));
+    }
+  }
+#endif
+  return 0;
 }
 
 // The primary file descriptor accessor. This object retains ownership of the

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();