LibLine: Change get_line to return a Result<String, Error>

This fixes a bunch of FIXME's in LibLine.
Also handles the case where read() would read zero bytes in vt_dsr() and
effectively block forever by erroring out.

Fixes #2370

Applications/Debugger/main.cpp

@@ -236,7 +236,13 @@ int main(int argc, char** argv)
         }
 
         for (;;) {
-            auto command = editor.get_line("(sdb) ");
+            auto command_result = editor.get_line("(sdb) ");
+
+            if (command_result.is_error())
+                return DebugSession::DebugDecision::Detach;
+
+            auto& command = command_result.value();
+
             bool success = false;
             Optional<DebugSession::DebugDecision> decision;
 

Libraries/LibLine/Editor.cpp

@@ -224,7 +224,7 @@ void Editor::suggest(size_t invariant_offset, size_t static_offset, Span::Mode o
     m_suggestion_manager.set_suggestion_variants(internal_static_offset, internal_invariant_offset, 0);
 }
 
-String Editor::get_line(const String& prompt)
+Result<String, Editor::Error> Editor::get_line(const String& prompt)
 {
     initialize();
     m_is_editing = true;
@@ -239,6 +239,9 @@ String Editor::get_line(const String& prompt)
         if (m_always_refresh)
             m_refresh_needed = true;
         refresh_display();
+        if (m_input_error.has_value()) {
+            return m_input_error.value();
+        }
         if (m_finish) {
             m_finish = false;
             printf("\n");
@@ -279,17 +282,18 @@ String Editor::get_line(const String& prompt)
                 m_refresh_needed = true;
                 continue;
             }
+
+            ScopedValueRollback errno_restorer(errno);
             perror("read failed");
-            // FIXME: exit()ing here is a bit off. Should communicate failure to caller somehow instead.
-            exit(2);
+
+            return Error::ReadFailure;
         }
 
         m_incomplete_data.append(keybuf, nread);
         nread = m_incomplete_data.size();
 
-        // FIXME: exit()ing here is a bit off. Should communicate failure to caller somehow instead.
         if (nread == 0)
-            exit(0);
+            return Error::Empty;
 
         auto reverse_tab = false;
         auto ctrl_held = false;
@@ -710,12 +714,19 @@ String Editor::get_line(const String& prompt)
                     fflush(stdout);
 
                     auto search_prompt = "\x1b[32msearch:\x1b[0m ";
-                    auto search_string = m_search_editor->get_line(search_prompt);
+                    auto search_string_result = m_search_editor->get_line(search_prompt);
 
                     m_search_editor = nullptr;
                     m_is_searching = false;
                     m_search_offset = 0;
 
+                    if (search_string_result.is_error()) {
+                        // Somethine broke, fail
+                        return search_string_result;
+                    }
+
+                    auto& search_string = search_string_result.value();
+
                     // Manually cleanup the search line.
                     reposition_cursor();
                     auto search_string_codepoint_length = Utf8View { search_string }.length_in_codepoints();
@@ -740,8 +751,9 @@ String Editor::get_line(const String& prompt)
             if (codepoint == m_termios.c_cc[VEOF]) {
                 if (m_buffer.is_empty()) {
                     printf("<EOF>\n");
-                    if (!m_always_refresh) // This is a little off, but it'll do for now
-                        exit(0);
+                    if (!m_always_refresh) {
+                        return Error::Eof;
+                    }
                 }
                 continue;
             }
@@ -1246,11 +1258,22 @@ Vector<size_t, 2> Editor::vt_dsr()
         (void)select(1, &readfds, nullptr, nullptr, &timeout);
         if (FD_ISSET(0, &readfds)) {
             auto nread = read(0, buf, 16);
+            if (nread < 0) {
+                m_input_error = Error::ReadFailure;
+                break;
+            }
+
+            if (nread == 0)
+                break;
+
             m_incomplete_data.append(buf, nread);
             more_junk_to_read = true;
         }
     } while (more_junk_to_read);
 
+    if (m_input_error.has_value())
+        return { 1, 1 };
+
     fputs("\033[6n", stdout);
     fflush(stdout);
 
@@ -1262,9 +1285,11 @@ Vector<size_t, 2> Editor::vt_dsr()
                 continue;
             }
             dbg() << "Error while reading DSR: " << strerror(errno);
+            m_input_error = Error::ReadFailure;
             return { 1, 1 };
         }
         if (nread == 0) {
+            m_input_error = Error::Empty;
             dbg() << "Terminal DSR issue; received no response";
             return { 1, 1 };
         }

Libraries/LibLine/Editor.h

@@ -33,6 +33,7 @@
 #include <AK/HashMap.h>
 #include <AK/NonnullOwnPtr.h>
 #include <AK/QuickSort.h>
+#include <AK/Result.h>
 #include <AK/String.h>
 #include <AK/Utf32View.h>
 #include <AK/Utf8View.h>
@@ -78,10 +79,16 @@ struct Configuration {
 
 class Editor {
 public:
+    enum class Error {
+        ReadFailure,
+        Empty,
+        Eof,
+    };
+
     explicit Editor(Configuration configuration = {});
     ~Editor();
 
-    String get_line(const String& prompt);
+    Result<String, Error> get_line(const String& prompt);
 
     void initialize()
     {
@@ -209,6 +216,7 @@ private:
         set_origin(0, 0);
         m_prompt_lines_at_suggestion_initiation = 0;
         m_refresh_needed = true;
+        m_input_error.clear();
     }
 
     void refresh_display();
@@ -276,8 +284,10 @@ private:
     Vector<u32, 1024> m_pre_search_buffer;
 
     Vector<u32, 1024> m_buffer;
-    Vector<char, 512> m_incomplete_data;
     ByteBuffer m_pending_chars;
+    Vector<char, 512> m_incomplete_data;
+    Optional<Error> m_input_error;
+
     size_t m_cursor { 0 };
     size_t m_drawn_cursor { 0 };
     size_t m_inline_search_cursor { 0 };

Shell/Shell.cpp

@@ -1701,19 +1701,29 @@ Vector<Line::CompletionSuggestion> Shell::complete(const Line::Editor& editor)
     return suggestions;
 }
 
-void Shell::read_single_line()
+bool Shell::read_single_line()
 {
-    auto line = editor.get_line(prompt());
+    auto line_result = editor.get_line(prompt());
+
+    if (line_result.is_error()) {
+        m_complete_line_builder.clear();
+        m_should_continue = ContinuationRequest::Nothing;
+        m_should_break_current_command = false;
+        Core::EventLoop::current().quit(line_result.error() == Line::Editor::Error::Eof ? 0 : 1);
+        return false;
+    }
+
+    auto& line = line_result.value();
 
     if (m_should_break_current_command) {
         m_complete_line_builder.clear();
         m_should_continue = ContinuationRequest::Nothing;
         m_should_break_current_command = false;
-        return;
+        return true;
     }
 
     if (line.is_empty())
-        return;
+        return true;
 
     // FIXME: This might be a bit counter-intuitive, since we put nothing
     //        between the two lines, even though the user has pressed enter
@@ -1725,17 +1735,18 @@ void Shell::read_single_line()
     m_should_continue = complete_or_exit_code.continuation;
 
     if (!complete_or_exit_code.has_value())
-        return;
+        return true;
 
     editor.add_to_history(m_complete_line_builder.build());
     m_complete_line_builder.clear();
+    return true;
 }
 
 void Shell::custom_event(Core::CustomEvent& event)
 {
     if (event.custom_type() == ReadLine) {
-        read_single_line();
-        Core::EventLoop::current().post_event(*this, make<Core::CustomEvent>(ShellEventType::ReadLine));
+        if (read_single_line())
+            Core::EventLoop::current().post_event(*this, make<Core::CustomEvent>(ShellEventType::ReadLine));
         return;
     }
 

Shell/Shell.h

@@ -123,7 +123,7 @@ public:
     bool should_read_more() const { return m_should_continue != ContinuationRequest::Nothing; }
     void finish_command() { m_should_break_current_command = true; }
 
-    void read_single_line();
+    bool read_single_line();
 
     struct termios termios;
     struct termios default_termios;

Userland/js.cpp

@@ -67,6 +67,7 @@ static bool s_dump_ast = false;
 static bool s_print_last_result = false;
 static OwnPtr<Line::Editor> s_editor;
 static int s_repl_line_level = 0;
+static bool s_fail_repl = false;
 
 static String prompt_for_level(int level)
 {
@@ -85,7 +86,14 @@ String read_next_piece()
     StringBuilder piece;
 
     do {
-        String line = s_editor->get_line(prompt_for_level(s_repl_line_level));
+        auto line_result = s_editor->get_line(prompt_for_level(s_repl_line_level));
+
+        if (line_result.is_error()) {
+            s_fail_repl = true;
+            return "";
+        }
+
+        auto& line = line_result.value();
         s_editor->add_to_history(line);
 
         piece.append(line);
@@ -369,7 +377,7 @@ JS::Value ReplObject::load_file(JS::Interpreter& interpreter)
 
 void repl(JS::Interpreter& interpreter)
 {
-    while (true) {
+    while (!s_fail_repl) {
         String piece = read_next_piece();
         if (piece.is_empty())
             continue;

Userland/test-crypto.cpp

@@ -113,7 +113,12 @@ int run(Function<void(const char*, size_t)> fn)
         Line::Editor editor;
         editor.initialize();
         for (;;) {
-            auto line = editor.get_line("> ");
+            auto line_result = editor.get_line("> ");
+
+            if (line_result.is_error())
+                break;
+            auto& line = line_result.value();
+
             if (line == ".wait") {
                 loop.exec();
             } else {