Author: Pavel Labath Date: 2020-10-14T14:55:03+02:00 New Revision: 6bb123b819c61c61197ec2ba54ceb6d16e9121cf
URL: https://github.com/llvm/llvm-project/commit/6bb123b819c61c61197ec2ba54ceb6d16e9121cf DIFF: https://github.com/llvm/llvm-project/commit/6bb123b819c61c61197ec2ba54ceb6d16e9121cf.diff LOG: [lldb] Modernize PseudoTerminal::OpenFirstAvailablePrimary replace char*+length combo with llvm::Error Added: Modified: 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 Removed: ################################################################################ diff --git a/lldb/include/lldb/Host/PseudoTerminal.h b/lldb/include/lldb/Host/PseudoTerminal.h index c2258f15e551..cf4f0c3769a5 100644 --- a/lldb/include/lldb/Host/PseudoTerminal.h +++ b/lldb/include/lldb/Host/PseudoTerminal.h @@ -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 @@ class PseudoTerminal { /// 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. /// diff --git a/lldb/source/Host/common/ProcessLaunchInfo.cpp b/lldb/source/Host/common/ProcessLaunchInfo.cpp index a4729a28ce74..851069d0f20f 100644 --- a/lldb/source/Host/common/ProcessLaunchInfo.cpp +++ b/lldb/source/Host/common/ProcessLaunchInfo.cpp @@ -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 diff --git a/lldb/source/Host/common/PseudoTerminal.cpp b/lldb/source/Host/common/PseudoTerminal.cpp index 4668b09f4fdb..8f0c3018363f 100644 --- a/lldb/source/Host/common/PseudoTerminal.cpp +++ b/lldb/source/Host/common/PseudoTerminal.cpp @@ -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; diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 9adf25f00b3e..40f6f6c21e26 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -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) diff --git a/lldb/unittests/Editline/CMakeLists.txt b/lldb/unittests/Editline/CMakeLists.txt index 220e263ce213..4b2643d15c5f 100644 --- a/lldb/unittests/Editline/CMakeLists.txt +++ b/lldb/unittests/Editline/CMakeLists.txt @@ -4,4 +4,5 @@ add_lldb_unittest(EditlineTests LINK_LIBS lldbHost lldbUtility + LLVMTestingSupport ) diff --git a/lldb/unittests/Editline/EditlineTest.cpp b/lldb/unittests/Editline/EditlineTest.cpp index 291ab3cb3687..1476dff0305b 100644 --- a/lldb/unittests/Editline/EditlineTest.cpp +++ b/lldb/unittests/Editline/EditlineTest.cpp @@ -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; diff --git a/lldb/unittests/Host/MainLoopTest.cpp b/lldb/unittests/Host/MainLoopTest.cpp index 9c9b6ae196cd..443ec6bd9f37 100644 --- a/lldb/unittests/Host/MainLoopTest.cpp +++ b/lldb/unittests/Host/MainLoopTest.cpp @@ -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); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
