[Lldb-commits] [lldb] r374576 - IOHandler: fall back on File::Read if a FILE* isn't available.
Lawrence D'Anna via lldb-commits
lldb-commits at lists.llvm.org
Fri Oct 11 10:43:32 PDT 2019
Author: lawrence_danna
Date: Fri Oct 11 10:43:32 2019
New Revision: 374576
URL: http://llvm.org/viewvc/llvm-project?rev=374576&view=rev
Log:
IOHandler: fall back on File::Read if a FILE* isn't available.
Summary:
IOHandler needs to read lines of input from a lldb::File.
The way it currently does this using, FILE*, which is something
we want to avoid now. I'd prefer to just replace the FILE* code
with calls to File::Read, but it contains an awkward and
delicate workaround specific to ctrl-C handling on windows, and
it's not clear if or how that workaround would translate to
lldb::File.
So in this patch, we use use the FILE* if it's available, and only
fall back on File::Read if that's the only option.
I think this is a reasonable approach here for two reasons. First
is that interactive terminal support is the one area where FILE*
can't be avoided. We need them for libedit and curses anyway,
and using them here as well is consistent with that pattern.
The second reason is that the comments express a hope that the
underlying windows bug that's being worked around will be fixed one
day, so hopefully when that happens, that whole path can be deleted.
Reviewers: JDevlieghere, jasonmolenda, labath, lanza
Reviewed By: labath
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D68622
Modified:
lldb/trunk/include/lldb/Core/IOHandler.h
lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
lldb/trunk/source/Core/IOHandler.cpp
Modified: lldb/trunk/include/lldb/Core/IOHandler.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/IOHandler.h?rev=374576&r1=374575&r2=374576&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Core/IOHandler.h (original)
+++ lldb/trunk/include/lldb/Core/IOHandler.h Fri Oct 11 10:43:32 2019
@@ -431,6 +431,7 @@ protected:
bool m_interrupt_exits;
bool m_editing; // Set to true when fetching a line manually (not using
// libedit)
+ std::string m_line_buffer;
};
// The order of base classes is important. Look at the constructor of
Modified: lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py?rev=374576&r1=374575&r2=374576&view=diff
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py Fri Oct 11 10:43:32 2019
@@ -399,9 +399,9 @@ class FileHandleTestCase(lldbtest.TestBa
@add_test_categories(['pyapi'])
- @expectedFailureAll() # FIXME IOHandler still using FILE*
+ @skipIf(py_version=['<', (3,)])
def test_string_inout(self):
- inf = io.StringIO("help help\n")
+ inf = io.StringIO("help help\np/x ~0\n")
outf = io.StringIO()
status = self.debugger.SetOutputFile(lldb.SBFile(outf))
self.assertTrue(status.Success())
@@ -412,10 +412,11 @@ class FileHandleTestCase(lldbtest.TestBa
self.debugger.GetOutputFile().Flush()
output = outf.getvalue()
self.assertIn('Show a list of all debugger commands', output)
+ self.assertIn('0xfff', output)
@add_test_categories(['pyapi'])
- @expectedFailureAll() # FIXME IOHandler still using FILE*
+ @skipIf(py_version=['<', (3,)])
def test_bytes_inout(self):
inf = io.BytesIO(b"help help\nhelp b\n")
outf = io.BytesIO()
Modified: lldb/trunk/source/Core/IOHandler.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/IOHandler.cpp?rev=374576&r1=374575&r2=374576&view=diff
==============================================================================
--- lldb/trunk/source/Core/IOHandler.cpp (original)
+++ lldb/trunk/source/Core/IOHandler.cpp Fri Oct 11 10:43:32 2019
@@ -70,6 +70,10 @@
using namespace lldb;
using namespace lldb_private;
+using llvm::None;
+using llvm::Optional;
+using llvm::StringRef;
+
IOHandler::IOHandler(Debugger &debugger, IOHandler::Type type)
: IOHandler(debugger, type,
@@ -306,94 +310,119 @@ void IOHandlerEditline::Deactivate() {
m_delegate.IOHandlerDeactivated(*this);
}
+// Split out a line from the buffer, if there is a full one to get.
+static Optional<std::string> SplitLine(std::string &line_buffer) {
+ size_t pos = line_buffer.find('\n');
+ if (pos == std::string::npos)
+ return None;
+ std::string line = StringRef(line_buffer.c_str(), pos).rtrim("\n\r");
+ line_buffer = line_buffer.substr(pos + 1);
+ return line;
+}
+
+// If the final line of the file ends without a end-of-line, return
+// it as a line anyway.
+static Optional<std::string> SplitLineEOF(std::string &line_buffer) {
+ if (llvm::all_of(line_buffer, isspace))
+ return None;
+ std::string line = std::move(line_buffer);
+ line_buffer.clear();
+ return line;
+}
+
bool IOHandlerEditline::GetLine(std::string &line, bool &interrupted) {
#ifndef LLDB_DISABLE_LIBEDIT
if (m_editline_up) {
bool b = m_editline_up->GetLine(line, interrupted);
- if (m_data_recorder)
+ if (b && m_data_recorder)
m_data_recorder->Record(line, true);
return b;
- } else {
+ }
#endif
- line.clear();
- FILE *in = GetInputFILE();
- if (in) {
- if (GetIsInteractive()) {
- const char *prompt = nullptr;
-
- if (m_multi_line && m_curr_line_idx > 0)
- prompt = GetContinuationPrompt();
-
- if (prompt == nullptr)
- prompt = GetPrompt();
-
- if (prompt && prompt[0]) {
- if (m_output_sp) {
- m_output_sp->Printf("%s", prompt);
- m_output_sp->Flush();
- }
- }
+ line.clear();
+
+ if (GetIsInteractive()) {
+ const char *prompt = nullptr;
+
+ if (m_multi_line && m_curr_line_idx > 0)
+ prompt = GetContinuationPrompt();
+
+ if (prompt == nullptr)
+ prompt = GetPrompt();
+
+ if (prompt && prompt[0]) {
+ if (m_output_sp) {
+ m_output_sp->Printf("%s", prompt);
+ m_output_sp->Flush();
}
- char buffer[256];
- bool done = false;
- bool got_line = false;
- m_editing = true;
- while (!done) {
+ }
+ }
+
+ Optional<std::string> got_line = SplitLine(m_line_buffer);
+
+ if (!got_line && !m_input_sp) {
+ // No more input file, we are done...
+ SetIsDone(true);
+ return false;
+ }
+
+ FILE *in = GetInputFILE();
+ char buffer[256];
+
+ if (!got_line && !in && m_input_sp) {
+ // there is no FILE*, fall back on just reading bytes from the stream.
+ while (!got_line) {
+ size_t bytes_read = sizeof(buffer);
+ Status error = m_input_sp->Read((void *)buffer, bytes_read);
+ if (error.Success() && !bytes_read) {
+ got_line = SplitLineEOF(m_line_buffer);
+ break;
+ }
+ if (error.Fail())
+ break;
+ m_line_buffer += StringRef(buffer, bytes_read);
+ got_line = SplitLine(m_line_buffer);
+ }
+ }
+
+ if (!got_line && in) {
+ m_editing = true;
+ while (!got_line) {
+ char *r = fgets(buffer, sizeof(buffer), in);
#ifdef _WIN32
- // ReadFile on Windows is supposed to set ERROR_OPERATION_ABORTED
- // according to the docs on MSDN. However, this has evidently been a
- // known bug since Windows 8. Therefore, we can't detect if a signal
- // interrupted in the fgets. So pressing ctrl-c causes the repl to end
- // and the process to exit. A temporary workaround is just to attempt to
- // fgets twice until this bug is fixed.
- if (fgets(buffer, sizeof(buffer), in) == nullptr &&
- fgets(buffer, sizeof(buffer), in) == nullptr) {
- // this is the equivalent of EINTR for Windows
- if (GetLastError() == ERROR_OPERATION_ABORTED)
- continue;
-#else
- if (fgets(buffer, sizeof(buffer), in) == nullptr) {
+ // ReadFile on Windows is supposed to set ERROR_OPERATION_ABORTED
+ // according to the docs on MSDN. However, this has evidently been a
+ // known bug since Windows 8. Therefore, we can't detect if a signal
+ // interrupted in the fgets. So pressing ctrl-c causes the repl to end
+ // and the process to exit. A temporary workaround is just to attempt to
+ // fgets twice until this bug is fixed.
+ if (r == nullptr)
+ r = fgets(buffer, sizeof(buffer), in);
+ // this is the equivalent of EINTR for Windows
+ if (r == nullptr && GetLastError() == ERROR_OPERATION_ABORTED)
+ continue;
#endif
- const int saved_errno = errno;
- if (feof(in))
- done = true;
- else if (ferror(in)) {
- if (saved_errno != EINTR)
- done = true;
- }
- } else {
- got_line = true;
- size_t buffer_len = strlen(buffer);
- assert(buffer[buffer_len] == '\0');
- char last_char = buffer[buffer_len - 1];
- if (last_char == '\r' || last_char == '\n') {
- done = true;
- // Strip trailing newlines
- while (last_char == '\r' || last_char == '\n') {
- --buffer_len;
- if (buffer_len == 0)
- break;
- last_char = buffer[buffer_len - 1];
- }
- }
- line.append(buffer, buffer_len);
- }
+ if (r == nullptr) {
+ if (ferror(in) && errno == EINTR)
+ continue;
+ if (feof(in))
+ got_line = SplitLineEOF(m_line_buffer);
+ break;
}
- m_editing = false;
- if (m_data_recorder && got_line)
- m_data_recorder->Record(line, true);
- // We might have gotten a newline on a line by itself make sure to return
- // true in this case.
- return got_line;
- } else {
- // No more input file, we are done...
- SetIsDone(true);
+ m_line_buffer += buffer;
+ got_line = SplitLine(m_line_buffer);
}
- return false;
-#ifndef LLDB_DISABLE_LIBEDIT
+ m_editing = false;
}
-#endif
+
+ if (got_line) {
+ line = got_line.getValue();
+ if (m_data_recorder)
+ m_data_recorder->Record(line, true);
+ }
+
+ return (bool)got_line;
}
#ifndef LLDB_DISABLE_LIBEDIT
More information about the lldb-commits
mailing list