Skip to content
This repository was archived by the owner on Jan 26, 2024. It is now read-only.

Commit 39f2b05

Browse files
mgornymgorny
committed
[lldb] [Host] Make Terminal methods return llvm::Error
Differential Revision: https://reviews.llvm.org/D111890
1 parent 8fbac4e commit 39f2b05

File tree

5 files changed

+90
-73
lines changed

5 files changed

+90
-73
lines changed

‎lldb/include/lldb/Host/Terminal.h

+16-8
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,12 @@
1212

1313
#include "lldb/Host/Config.h"
1414
#include "lldb/lldb-private.h"
15+
#include "llvm/Support/Error.h"
1516

1617
namespace lldb_private {
1718

19+
class TerminalState;
20+
1821
class Terminal {
1922
public:
2023
Terminal(int fd = -1) : m_fd(fd) {}
@@ -31,12 +34,19 @@ class Terminal {
3134

3235
void Clear() { m_fd = -1; }
3336

34-
bool SetEcho(bool enabled);
37+
llvm::Error SetEcho(bool enabled);
3538

36-
bool SetCanonical(bool enabled);
39+
llvm::Error SetCanonical(bool enabled);
3740

3841
protected:
42+
struct Data;
43+
3944
int m_fd; // This may or may not be a terminal file descriptor
45+
46+
llvm::Expected<Data> GetData();
47+
llvm::Error SetData(const Data &data);
48+
49+
friend class TerminalState;
4050
};
4151

4252
/// \class TerminalState Terminal.h "lldb/Host/Terminal.h"
@@ -45,8 +55,6 @@ class Terminal {
4555
/// This class can be used to remember the terminal state for a file
4656
/// descriptor and later restore that state as it originally was.
4757
class TerminalState {
48-
struct Data;
49-
5058
public:
5159
/// Construct a new instance and optionally save terminal state.
5260
///
@@ -125,10 +133,10 @@ class TerminalState {
125133
bool ProcessGroupIsValid() const;
126134

127135
// Member variables
128-
Terminal m_tty; ///< A terminal
129-
int m_tflags = -1; ///< Cached tflags information.
130-
std::unique_ptr<Data> m_data; ///< Platform-specific implementation.
131-
lldb::pid_t m_process_group = -1; ///< Cached process group information.
136+
Terminal m_tty; ///< A terminal
137+
int m_tflags = -1; ///< Cached tflags information.
138+
std::unique_ptr<Terminal::Data> m_data; ///< Platform-specific implementation.
139+
lldb::pid_t m_process_group = -1; ///< Cached process group information.
132140
};
133141

134142
} // namespace lldb_private

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

+64-57
Original file line numberDiff line numberDiff line change
@@ -21,71 +21,78 @@
2121

2222
using namespace lldb_private;
2323

24+
struct Terminal::Data {
25+
#if LLDB_ENABLE_TERMIOS
26+
struct termios m_termios; ///< Cached terminal state information.
27+
#endif
28+
};
29+
2430
bool Terminal::IsATerminal() const { return m_fd >= 0 && ::isatty(m_fd); }
2531

26-
bool Terminal::SetEcho(bool enabled) {
27-
if (FileDescriptorIsValid()) {
32+
llvm::Expected<Terminal::Data> Terminal::GetData() {
33+
if (!FileDescriptorIsValid())
34+
return llvm::createStringError(llvm::inconvertibleErrorCode(),
35+
"invalid fd");
36+
2837
#if LLDB_ENABLE_TERMIOS
29-
if (IsATerminal()) {
30-
struct termios fd_termios;
31-
if (::tcgetattr(m_fd, &fd_termios) == 0) {
32-
bool set_corectly = false;
33-
if (enabled) {
34-
if (fd_termios.c_lflag & ECHO)
35-
set_corectly = true;
36-
else
37-
fd_termios.c_lflag |= ECHO;
38-
} else {
39-
if (fd_termios.c_lflag & ECHO)
40-
fd_termios.c_lflag &= ~ECHO;
41-
else
42-
set_corectly = true;
43-
}
44-
45-
if (set_corectly)
46-
return true;
47-
return ::tcsetattr(m_fd, TCSANOW, &fd_termios) == 0;
48-
}
49-
}
50-
#endif // #if LLDB_ENABLE_TERMIOS
51-
}
52-
return false;
38+
if (!IsATerminal())
39+
return llvm::createStringError(llvm::inconvertibleErrorCode(),
40+
"fd not a terminal");
41+
42+
Data data;
43+
if (::tcgetattr(m_fd, &data.m_termios) != 0)
44+
return llvm::createStringError(
45+
std::error_code(errno, std::generic_category()),
46+
"unable to get teletype attributes");
47+
return data;
48+
#else // !LLDB_ENABLE_TERMIOS
49+
return llvm::createStringError(llvm::inconvertibleErrorCode(),
50+
"termios support missing in LLDB");
51+
#endif // LLDB_ENABLE_TERMIOS
5352
}
5453

55-
bool Terminal::SetCanonical(bool enabled) {
56-
if (FileDescriptorIsValid()) {
54+
llvm::Error Terminal::SetData(const Terminal::Data &data) {
5755
#if LLDB_ENABLE_TERMIOS
58-
if (IsATerminal()) {
59-
struct termios fd_termios;
60-
if (::tcgetattr(m_fd, &fd_termios) == 0) {
61-
bool set_corectly = false;
62-
if (enabled) {
63-
if (fd_termios.c_lflag & ICANON)
64-
set_corectly = true;
65-
else
66-
fd_termios.c_lflag |= ICANON;
67-
} else {
68-
if (fd_termios.c_lflag & ICANON)
69-
fd_termios.c_lflag &= ~ICANON;
70-
else
71-
set_corectly = true;
72-
}
73-
74-
if (set_corectly)
75-
return true;
76-
return ::tcsetattr(m_fd, TCSANOW, &fd_termios) == 0;
77-
}
78-
}
79-
#endif // #if LLDB_ENABLE_TERMIOS
80-
}
81-
return false;
56+
assert(FileDescriptorIsValid());
57+
assert(IsATerminal());
58+
59+
if (::tcsetattr(m_fd, TCSANOW, &data.m_termios) != 0)
60+
return llvm::createStringError(
61+
std::error_code(errno, std::generic_category()),
62+
"unable to set teletype attributes");
63+
return llvm::Error::success();
64+
#else // !LLDB_ENABLE_TERMIOS
65+
llvm_unreachable("SetData() should not be called if !LLDB_ENABLE_TERMIOS");
66+
#endif // LLDB_ENABLE_TERMIOS
8267
}
8368

84-
struct TerminalState::Data {
69+
llvm::Error Terminal::SetEcho(bool enabled) {
70+
llvm::Expected<Data> data = GetData();
71+
if (!data)
72+
return data.takeError();
73+
8574
#if LLDB_ENABLE_TERMIOS
86-
struct termios m_termios; ///< Cached terminal state information.
87-
#endif
88-
};
75+
struct termios &fd_termios = data->m_termios;
76+
fd_termios.c_lflag &= ~ECHO;
77+
if (enabled)
78+
fd_termios.c_lflag |= ECHO;
79+
return SetData(data.get());
80+
#endif // LLDB_ENABLE_TERMIOS
81+
}
82+
83+
llvm::Error Terminal::SetCanonical(bool enabled) {
84+
llvm::Expected<Data> data = GetData();
85+
if (!data)
86+
return data.takeError();
87+
88+
#if LLDB_ENABLE_TERMIOS
89+
struct termios &fd_termios = data->m_termios;
90+
fd_termios.c_lflag &= ~ICANON;
91+
if (enabled)
92+
fd_termios.c_lflag |= ICANON;
93+
return SetData(data.get());
94+
#endif // LLDB_ENABLE_TERMIOS
95+
}
8996

9097
TerminalState::TerminalState(Terminal term, bool save_process_group)
9198
: m_tty(term) {
@@ -109,7 +116,7 @@ bool TerminalState::Save(Terminal term, bool save_process_group) {
109116
#if LLDB_ENABLE_POSIX
110117
m_tflags = ::fcntl(fd, F_GETFL, 0);
111118
#if LLDB_ENABLE_TERMIOS
112-
std::unique_ptr<Data> new_data{new Data()};
119+
std::unique_ptr<Terminal::Data> new_data{new Terminal::Data()};
113120
if (::tcgetattr(fd, &new_data->m_termios) == 0)
114121
m_data = std::move(new_data);
115122
#endif // LLDB_ENABLE_TERMIOS

‎lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -435,8 +435,9 @@ class IOHandlerPythonInterpreter : public IOHandler {
435435
TerminalState terminal_state(terminal);
436436

437437
if (terminal.IsATerminal()) {
438-
terminal.SetCanonical(false);
439-
terminal.SetEcho(true);
438+
// FIXME: error handling?
439+
llvm::consumeError(terminal.SetCanonical(false));
440+
llvm::consumeError(terminal.SetEcho(true));
440441
}
441442

442443
ScriptInterpreterPythonImpl::Locker locker(

‎lldb/source/Target/Process.cpp

+3-2
Original file line numberDiff line numberDiff line change
@@ -4349,8 +4349,9 @@ class IOHandlerProcessSTDIO : public IOHandler {
43494349
const int read_fd = m_read_file.GetDescriptor();
43504350
Terminal terminal(read_fd);
43514351
TerminalState terminal_state(terminal, false);
4352-
terminal.SetCanonical(false);
4353-
terminal.SetEcho(false);
4352+
// FIXME: error handling?
4353+
llvm::consumeError(terminal.SetCanonical(false));
4354+
llvm::consumeError(terminal.SetEcho(false));
43544355
// FD_ZERO, FD_SET are not supported on windows
43554356
#ifndef _WIN32
43564357
const int pipe_read_fd = m_pipe.GetReadFileDescriptor();

‎lldb/unittests/Host/posix/TerminalTest.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -51,23 +51,23 @@ TEST_F(TerminalTest, PipeIsNotATerminal) {
5151
TEST_F(TerminalTest, SetEcho) {
5252
struct termios terminfo;
5353

54-
ASSERT_EQ(m_term.SetEcho(true), true);
54+
ASSERT_THAT_ERROR(m_term.SetEcho(true), llvm::Succeeded());
5555
ASSERT_EQ(tcgetattr(m_fd, &terminfo), 0);
5656
EXPECT_NE(terminfo.c_lflag & ECHO, 0U);
5757

58-
ASSERT_EQ(m_term.SetEcho(false), true);
58+
ASSERT_THAT_ERROR(m_term.SetEcho(false), llvm::Succeeded());
5959
ASSERT_EQ(tcgetattr(m_fd, &terminfo), 0);
6060
EXPECT_EQ(terminfo.c_lflag & ECHO, 0U);
6161
}
6262

6363
TEST_F(TerminalTest, SetCanonical) {
6464
struct termios terminfo;
6565

66-
ASSERT_EQ(m_term.SetCanonical(true), true);
66+
ASSERT_THAT_ERROR(m_term.SetCanonical(true), llvm::Succeeded());
6767
ASSERT_EQ(tcgetattr(m_fd, &terminfo), 0);
6868
EXPECT_NE(terminfo.c_lflag & ICANON, 0U);
6969

70-
ASSERT_EQ(m_term.SetCanonical(false), true);
70+
ASSERT_THAT_ERROR(m_term.SetCanonical(false), llvm::Succeeded());
7171
ASSERT_EQ(tcgetattr(m_fd, &terminfo), 0);
7272
EXPECT_EQ(terminfo.c_lflag & ICANON, 0U);
7373
}

0 commit comments

Comments
 (0)