[Lldb-commits] [lldb] 6bb123b - [lldb] Modernize PseudoTerminal::OpenFirstAvailablePrimary
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Wed Oct 14 06:02:26 PDT 2020
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);
More information about the lldb-commits
mailing list