Skip to content

Commit 6bb123b

Browse files
labathlabath
committed
[lldb] Modernize PseudoTerminal::OpenFirstAvailablePrimary
replace char*+length combo with llvm::Error
1 parent cde06f7 commit 6bb123b

File tree

7 files changed

+58
-93
lines changed

7 files changed

+58
-93
lines changed

‎lldb/include/lldb/Host/PseudoTerminal.h

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@
99
#ifndef LLDB_HOST_PSEUDOTERMINAL_H
1010
#define LLDB_HOST_PSEUDOTERMINAL_H
1111

12+
#include "lldb/lldb-defines.h"
13+
#include "llvm/Support/Error.h"
1214
#include <fcntl.h>
1315
#include <string>
1416

15-
#include "lldb/lldb-defines.h"
16-
1717
namespace lldb_private {
1818

1919
/// \class PseudoTerminal PseudoTerminal.h "lldb/Host/PseudoTerminal.h"
@@ -128,18 +128,9 @@ class PseudoTerminal {
128128
/// Flags to use when calling \c posix_openpt(\a oflag).
129129
/// A value of "O_RDWR|O_NOCTTY" is suggested.
130130
///
131-
/// \param[out] error_str
132-
/// An pointer to an error that can describe any errors that
133-
/// occur. This can be NULL if no error status is desired.
134-
///
135-
/// \return
136-
/// \b true when the primary files descriptor is
137-
/// successfully opened.
138-
/// \b false if anything goes wrong.
139-
///
140131
/// \see PseudoTerminal::GetPrimaryFileDescriptor() @see
141132
/// PseudoTerminal::ReleasePrimaryFileDescriptor()
142-
bool OpenFirstAvailablePrimary(int oflag, char *error_str, size_t error_len);
133+
llvm::Error OpenFirstAvailablePrimary(int oflag);
143134

144135
/// Open the secondary for the current primary pseudo terminal.
145136
///

‎lldb/source/Host/common/ProcessLaunchInfo.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -218,10 +218,9 @@ llvm::Error ProcessLaunchInfo::SetUpPtyRedirection() {
218218
// do for now.
219219
open_flags |= O_CLOEXEC;
220220
#endif
221-
if (!m_pty->OpenFirstAvailablePrimary(open_flags, nullptr, 0)) {
222-
return llvm::createStringError(llvm::inconvertibleErrorCode(),
223-
"PTY::OpenFirstAvailablePrimary failed");
224-
}
221+
if (llvm::Error Err = m_pty->OpenFirstAvailablePrimary(open_flags))
222+
return Err;
223+
225224
const FileSpec secondary_file_spec(m_pty->GetSecondaryName());
226225

227226
// Only use the secondary tty if we don't have anything specified for

‎lldb/source/Host/common/PseudoTerminal.cpp

Lines changed: 46 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -65,52 +65,32 @@ void PseudoTerminal::CloseSecondaryFileDescriptor() {
6565
}
6666
}
6767

68-
// Open the first available pseudo terminal with OFLAG as the permissions. The
69-
// file descriptor is stored in this object and can be accessed with the
70-
// PrimaryFileDescriptor() accessor. The ownership of the primary file
71-
// descriptor can be released using the ReleasePrimaryFileDescriptor() accessor.
72-
// If this object has a valid primary files descriptor when its destructor is
73-
// called, it will close the primary file descriptor, therefore clients must
74-
// call ReleasePrimaryFileDescriptor() if they wish to use the primary file
75-
// descriptor after this object is out of scope or destroyed.
76-
//
77-
// RETURNS:
78-
// True when successful, false indicating an error occurred.
79-
bool PseudoTerminal::OpenFirstAvailablePrimary(int oflag, char *error_str,
80-
size_t error_len) {
81-
if (error_str)
82-
error_str[0] = '\0';
83-
68+
llvm::Error PseudoTerminal::OpenFirstAvailablePrimary(int oflag) {
8469
#if LLDB_ENABLE_POSIX
8570
// Open the primary side of a pseudo terminal
8671
m_primary_fd = ::posix_openpt(oflag);
8772
if (m_primary_fd < 0) {
88-
if (error_str)
89-
ErrnoToStr(error_str, error_len);
90-
return false;
73+
return llvm::errorCodeToError(
74+
std::error_code(errno, std::generic_category()));
9175
}
9276

9377
// Grant access to the secondary pseudo terminal
9478
if (::grantpt(m_primary_fd) < 0) {
95-
if (error_str)
96-
ErrnoToStr(error_str, error_len);
79+
std::error_code EC(errno, std::generic_category());
9780
ClosePrimaryFileDescriptor();
98-
return false;
81+
return llvm::errorCodeToError(EC);
9982
}
10083

10184
// Clear the lock flag on the secondary pseudo terminal
10285
if (::unlockpt(m_primary_fd) < 0) {
103-
if (error_str)
104-
ErrnoToStr(error_str, error_len);
86+
std::error_code EC(errno, std::generic_category());
10587
ClosePrimaryFileDescriptor();
106-
return false;
88+
return llvm::errorCodeToError(EC);
10789
}
10890

109-
return true;
91+
return llvm::Error::success();
11092
#else
111-
if (error_str)
112-
::snprintf(error_str, error_len, "%s", "pseudo terminal not supported");
113-
return false;
93+
return llvm::errorCodeToError(llvm::errc::not_supported);
11494
#endif
11595
}
11696

@@ -180,54 +160,53 @@ lldb::pid_t PseudoTerminal::Fork(char *error_str, size_t error_len) {
180160
error_str[0] = '\0';
181161
pid_t pid = LLDB_INVALID_PROCESS_ID;
182162
#if LLDB_ENABLE_POSIX
183-
int flags = O_RDWR;
184-
flags |= O_CLOEXEC;
185-
if (OpenFirstAvailablePrimary(flags, error_str, error_len)) {
186-
// Successfully opened our primary pseudo terminal
163+
if (llvm::Error Err = OpenFirstAvailablePrimary(O_RDWR | O_CLOEXEC)) {
164+
snprintf(error_str, error_len, "%s", toString(std::move(Err)).c_str());
165+
return LLDB_INVALID_PROCESS_ID;
166+
}
187167

188-
pid = ::fork();
189-
if (pid < 0) {
190-
// Fork failed
191-
if (error_str)
192-
ErrnoToStr(error_str, error_len);
193-
} else if (pid == 0) {
194-
// Child Process
195-
::setsid();
168+
pid = ::fork();
169+
if (pid < 0) {
170+
// Fork failed
171+
if (error_str)
172+
ErrnoToStr(error_str, error_len);
173+
} else if (pid == 0) {
174+
// Child Process
175+
::setsid();
196176

197-
if (OpenSecondary(O_RDWR, error_str, error_len)) {
198-
// Successfully opened secondary
177+
if (OpenSecondary(O_RDWR, error_str, error_len)) {
178+
// Successfully opened secondary
199179

200-
// Primary FD should have O_CLOEXEC set, but let's close it just in
201-
// case...
202-
ClosePrimaryFileDescriptor();
180+
// Primary FD should have O_CLOEXEC set, but let's close it just in
181+
// case...
182+
ClosePrimaryFileDescriptor();
203183

204184
#if defined(TIOCSCTTY)
205-
// Acquire the controlling terminal
206-
if (::ioctl(m_secondary_fd, TIOCSCTTY, (char *)0) < 0) {
207-
if (error_str)
208-
ErrnoToStr(error_str, error_len);
209-
}
185+
// Acquire the controlling terminal
186+
if (::ioctl(m_secondary_fd, TIOCSCTTY, (char *)0) < 0) {
187+
if (error_str)
188+
ErrnoToStr(error_str, error_len);
189+
}
210190
#endif
211-
// Duplicate all stdio file descriptors to the secondary pseudo terminal
212-
if (::dup2(m_secondary_fd, STDIN_FILENO) != STDIN_FILENO) {
213-
if (error_str && !error_str[0])
214-
ErrnoToStr(error_str, error_len);
215-
}
191+
// Duplicate all stdio file descriptors to the secondary pseudo terminal
192+
if (::dup2(m_secondary_fd, STDIN_FILENO) != STDIN_FILENO) {
193+
if (error_str && !error_str[0])
194+
ErrnoToStr(error_str, error_len);
195+
}
216196

217-
if (::dup2(m_secondary_fd, STDOUT_FILENO) != STDOUT_FILENO) {
218-
if (error_str && !error_str[0])
219-
ErrnoToStr(error_str, error_len);
220-
}
197+
if (::dup2(m_secondary_fd, STDOUT_FILENO) != STDOUT_FILENO) {
198+
if (error_str && !error_str[0])
199+
ErrnoToStr(error_str, error_len);
200+
}
221201

222-
if (::dup2(m_secondary_fd, STDERR_FILENO) != STDERR_FILENO) {
223-
if (error_str && !error_str[0])
224-
ErrnoToStr(error_str, error_len);
225-
}
202+
if (::dup2(m_secondary_fd, STDERR_FILENO) != STDERR_FILENO) {
203+
if (error_str && !error_str[0])
204+
ErrnoToStr(error_str, error_len);
226205
}
227-
} else {
228-
// Parent Process
229-
// Do nothing and let the pid get returned!
230206
}
207+
} else {
208+
// Parent Process
209+
// Do nothing and let the pid get returned!
231210
}
232211
#endif
233212
return pid;

‎lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -817,7 +817,7 @@ Status ProcessGDBRemote::DoLaunch(lldb_private::Module *exe_module,
817817
// since 'O' packets can really slow down debugging if the inferior
818818
// does a lot of output.
819819
if ((!stdin_file_spec || !stdout_file_spec || !stderr_file_spec) &&
820-
pty.OpenFirstAvailablePrimary(O_RDWR | O_NOCTTY, nullptr, 0)) {
820+
!errorToBool(pty.OpenFirstAvailablePrimary(O_RDWR | O_NOCTTY))) {
821821
FileSpec secondary_name(pty.GetSecondaryName());
822822

823823
if (!stdin_file_spec)

‎lldb/unittests/Editline/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,5 @@ add_lldb_unittest(EditlineTests
44
LINK_LIBS
55
lldbHost
66
lldbUtility
7+
LLVMTestingSupport
78
)

‎lldb/unittests/Editline/EditlineTest.cpp

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -99,21 +99,16 @@ EditlineAdapter::EditlineAdapter()
9999
lldb_private::Status error;
100100

101101
// Open the first master pty available.
102-
char error_string[256];
103-
error_string[0] = '\0';
104-
if (!_pty.OpenFirstAvailablePrimary(O_RDWR, error_string,
105-
sizeof(error_string))) {
106-
fprintf(stderr, "failed to open first available master pty: '%s'\n",
107-
error_string);
108-
return;
109-
}
102+
EXPECT_THAT_ERROR(_pty.OpenFirstAvailablePrimary(O_RDWR), llvm::Succeeded());
110103

111104
// Grab the master fd. This is a file descriptor we will:
112105
// (1) write to when we want to send input to editline.
113106
// (2) read from when we want to see what editline sends back.
114107
_pty_master_fd = _pty.GetPrimaryFileDescriptor();
115108

116109
// Open the corresponding secondary pty.
110+
char error_string[256];
111+
error_string[0] = '\0';
117112
if (!_pty.OpenSecondary(O_RDWR, error_string, sizeof(error_string))) {
118113
fprintf(stderr, "failed to open secondary pty: '%s'\n", error_string);
119114
return;

‎lldb/unittests/Host/MainLoopTest.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ TEST_F(MainLoopTest, TerminatesImmediately) {
102102
TEST_F(MainLoopTest, DetectsEOF) {
103103

104104
PseudoTerminal term;
105-
ASSERT_TRUE(term.OpenFirstAvailablePrimary(O_RDWR, nullptr, 0));
105+
ASSERT_THAT_ERROR(term.OpenFirstAvailablePrimary(O_RDWR), llvm::Succeeded());
106106
ASSERT_TRUE(term.OpenSecondary(O_RDWR | O_NOCTTY, nullptr, 0));
107107
auto conn = std::make_unique<ConnectionFileDescriptor>(
108108
term.ReleasePrimaryFileDescriptor(), true);

0 commit comments

Comments
 (0)