[lldb] Modernize PseudoTerminal::OpenFirstAvailablePrimary
authorPavel Labath <pavel@labath.sk>
Wed, 7 Oct 2020 15:35:13 +0000 (17:35 +0200)
committerPavel Labath <pavel@labath.sk>
Wed, 14 Oct 2020 12:55:03 +0000 (14:55 +0200)
replace char*+length combo with llvm::Error

lldb/include/lldb/Host/PseudoTerminal.h
lldb/source/Host/common/ProcessLaunchInfo.cpp
lldb/source/Host/common/PseudoTerminal.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/unittests/Editline/CMakeLists.txt
lldb/unittests/Editline/EditlineTest.cpp
lldb/unittests/Host/MainLoopTest.cpp

index c2258f1..cf4f0c3 100644 (file)
@@ -9,11 +9,11 @@
 #ifndef LLDB_HOST_PSEUDOTERMINAL_H
 #define LLDB_HOST_PSEUDOTERMINAL_H
 
+#include "lldb/lldb-defines.h"
+#include "llvm/Support/Error.h"
 #include <fcntl.h>
 #include <string>
 
-#include "lldb/lldb-defines.h"
-
 namespace lldb_private {
 
 /// \class PseudoTerminal PseudoTerminal.h "lldb/Host/PseudoTerminal.h"
@@ -128,18 +128,9 @@ public:
   ///     Flags to use when calling \c posix_openpt(\a oflag).
   ///     A value of "O_RDWR|O_NOCTTY" is suggested.
   ///
-  /// \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 true when the primary files descriptor is
-  ///         successfully opened.
-  ///     \b false if anything goes wrong.
-  ///
   /// \see PseudoTerminal::GetPrimaryFileDescriptor() @see
   /// PseudoTerminal::ReleasePrimaryFileDescriptor()
-  bool OpenFirstAvailablePrimary(int oflag, char *error_str, size_t error_len);
+  llvm::Error OpenFirstAvailablePrimary(int oflag);
 
   /// Open the secondary for the current primary pseudo terminal.
   ///
index a4729a2..851069d 100644 (file)
@@ -218,10 +218,9 @@ llvm::Error ProcessLaunchInfo::SetUpPtyRedirection() {
   // do for now.
   open_flags |= O_CLOEXEC;
 #endif
-  if (!m_pty->OpenFirstAvailablePrimary(open_flags, nullptr, 0)) {
-    return llvm::createStringError(llvm::inconvertibleErrorCode(),
-                                   "PTY::OpenFirstAvailablePrimary failed");
-  }
+  if (llvm::Error Err = m_pty->OpenFirstAvailablePrimary(open_flags))
+    return Err;
+
   const FileSpec secondary_file_spec(m_pty->GetSecondaryName());
 
   // Only use the secondary tty if we don't have anything specified for
index 4668b09..8f0c301 100644 (file)
@@ -65,52 +65,32 @@ void PseudoTerminal::CloseSecondaryFileDescriptor() {
   }
 }
 
-// Open the first available pseudo terminal with OFLAG as the permissions. The
-// file descriptor is stored in this object and can be accessed with the
-// PrimaryFileDescriptor() accessor. The ownership of the primary file
-// descriptor can be released using the ReleasePrimaryFileDescriptor() accessor.
-// If this object has a valid primary files descriptor when its destructor is
-// called, it will close the primary file descriptor, therefore clients must
-// call ReleasePrimaryFileDescriptor() if they wish to use the primary file
-// descriptor after this object is out of scope or destroyed.
-//
-// RETURNS:
-//  True when successful, false indicating an error occurred.
-bool PseudoTerminal::OpenFirstAvailablePrimary(int oflag, char *error_str,
-                                               size_t error_len) {
-  if (error_str)
-    error_str[0] = '\0';
-
+llvm::Error PseudoTerminal::OpenFirstAvailablePrimary(int oflag) {
 #if LLDB_ENABLE_POSIX
   // Open the primary side of a pseudo terminal
   m_primary_fd = ::posix_openpt(oflag);
   if (m_primary_fd < 0) {
-    if (error_str)
-      ErrnoToStr(error_str, error_len);
-    return false;
+    return llvm::errorCodeToError(
+        std::error_code(errno, std::generic_category()));
   }
 
   // Grant access to the secondary pseudo terminal
   if (::grantpt(m_primary_fd) < 0) {
-    if (error_str)
-      ErrnoToStr(error_str, error_len);
+    std::error_code EC(errno, std::generic_category());
     ClosePrimaryFileDescriptor();
-    return false;
+    return llvm::errorCodeToError(EC);
   }
 
   // Clear the lock flag on the secondary pseudo terminal
   if (::unlockpt(m_primary_fd) < 0) {
-    if (error_str)
-      ErrnoToStr(error_str, error_len);
+    std::error_code EC(errno, std::generic_category());
     ClosePrimaryFileDescriptor();
-    return false;
+    return llvm::errorCodeToError(EC);
   }
 
-  return true;
+  return llvm::Error::success();
 #else
-  if (error_str)
-    ::snprintf(error_str, error_len, "%s", "pseudo terminal not supported");
-  return false;
+  return llvm::errorCodeToError(llvm::errc::not_supported);
 #endif
 }
 
@@ -180,54 +160,53 @@ lldb::pid_t PseudoTerminal::Fork(char *error_str, size_t error_len) {
     error_str[0] = '\0';
   pid_t pid = LLDB_INVALID_PROCESS_ID;
 #if LLDB_ENABLE_POSIX
-  int flags = O_RDWR;
-  flags |= O_CLOEXEC;
-  if (OpenFirstAvailablePrimary(flags, error_str, error_len)) {
-    // Successfully opened our primary pseudo terminal
+  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;
+  }
 
-    pid = ::fork();
-    if (pid < 0) {
-      // Fork failed
-      if (error_str)
-        ErrnoToStr(error_str, error_len);
-    } else if (pid == 0) {
-      // Child Process
-      ::setsid();
+  pid = ::fork();
+  if (pid < 0) {
+    // Fork failed
+    if (error_str)
+      ErrnoToStr(error_str, error_len);
+  } else if (pid == 0) {
+    // Child Process
+    ::setsid();
 
-      if (OpenSecondary(O_RDWR, error_str, error_len)) {
-        // Successfully opened secondary
+    if (OpenSecondary(O_RDWR, error_str, error_len)) {
+      // Successfully opened secondary
 
-        // 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);
-        }
+      // 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);
-        }
+      // 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, 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);
-        }
+      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!
     }
+  } else {
+    // Parent Process
+    // Do nothing and let the pid get returned!
   }
 #endif
   return pid;
index 9adf25f..40f6f6c 100644 (file)
@@ -817,7 +817,7 @@ Status ProcessGDBRemote::DoLaunch(lldb_private::Module *exe_module,
         // since 'O' packets can really slow down debugging if the inferior
         // does a lot of output.
         if ((!stdin_file_spec || !stdout_file_spec || !stderr_file_spec) &&
-            pty.OpenFirstAvailablePrimary(O_RDWR | O_NOCTTY, nullptr, 0)) {
+            !errorToBool(pty.OpenFirstAvailablePrimary(O_RDWR | O_NOCTTY))) {
           FileSpec secondary_name(pty.GetSecondaryName());
 
           if (!stdin_file_spec)
index 220e263..4b2643d 100644 (file)
@@ -4,4 +4,5 @@ add_lldb_unittest(EditlineTests
   LINK_LIBS
     lldbHost
     lldbUtility
+    LLVMTestingSupport
   )
index 291ab3c..1476dff 100644 (file)
@@ -99,14 +99,7 @@ EditlineAdapter::EditlineAdapter()
   lldb_private::Status error;
 
   // Open the first master pty available.
-  char error_string[256];
-  error_string[0] = '\0';
-  if (!_pty.OpenFirstAvailablePrimary(O_RDWR, error_string,
-                                      sizeof(error_string))) {
-    fprintf(stderr, "failed to open first available master pty: '%s'\n",
-            error_string);
-    return;
-  }
+  EXPECT_THAT_ERROR(_pty.OpenFirstAvailablePrimary(O_RDWR), llvm::Succeeded());
 
   // Grab the master fd.  This is a file descriptor we will:
   // (1) write to when we want to send input to editline.
@@ -114,6 +107,8 @@ EditlineAdapter::EditlineAdapter()
   _pty_master_fd = _pty.GetPrimaryFileDescriptor();
 
   // Open the corresponding secondary pty.
+  char error_string[256];
+  error_string[0] = '\0';
   if (!_pty.OpenSecondary(O_RDWR, error_string, sizeof(error_string))) {
     fprintf(stderr, "failed to open secondary pty: '%s'\n", error_string);
     return;
index 9c9b6ae..443ec6b 100644 (file)
@@ -102,7 +102,7 @@ TEST_F(MainLoopTest, TerminatesImmediately) {
 TEST_F(MainLoopTest, DetectsEOF) {
 
   PseudoTerminal term;
-  ASSERT_TRUE(term.OpenFirstAvailablePrimary(O_RDWR, nullptr, 0));
+  ASSERT_THAT_ERROR(term.OpenFirstAvailablePrimary(O_RDWR), llvm::Succeeded());
   ASSERT_TRUE(term.OpenSecondary(O_RDWR | O_NOCTTY, nullptr, 0));
   auto conn = std::make_unique<ConnectionFileDescriptor>(
       term.ReleasePrimaryFileDescriptor(), true);