[LLDB][NFC] Reliability fixes for IOHandlerCursesGUI

[fixathon]

Summary

• checking retval of function calls
• signed/unsigned mismatch
• dead code removal
• null dereference fix

[clayborg]

lldb/source/Core/IOHandlerCursesGUI.cpp

391–394

the underlying curses functions return "int". Were there places where people were comparing them to unsigned?

[fixathon]

lldb/source/Core/IOHandlerCursesGUI.cpp

391–394

A function HorizontalLine() at line 2582 requires non-negative input. Good point about the underlying function possibly returning a negative still.

2582

Here we have HorizontalLine() function that requires non-negative input

[fixathon]

Responded to comments

lldb/source/Core/IOHandlerCursesGUI.cpp

391–394

I took another look, and I am not sure what the complaint is. Both the HorizontalLine() and its underlying call to ::whline take input typed as "signed int". Perhaps ::whline is unable to handle negative input?

void HorizontalLine(int n, chtype h_char = ACS_HLINE) {
  ::whline(m_window, h_char, n);
}

[clayborg]

lldb/source/Core/IOHandlerCursesGUI.cpp

391–394

Not sure either. I don't want to change these as they should return "int" like the original API.

2582

But surface.HorizontalLine(...) function takes an "int", and now we are passing it a uint32_t. If this function requires a >= 0 value, it should do the check internally

[fixathon]

Address review comments about the signed/unsigned issue

[fixathon]

Addressed the comments. Will deal with the signed/unsigned issue in a separate diff if still needed.

lldb/source/Core/IOHandlerCursesGUI.cpp

2582

I agree. I've reverted the function signatures back to signed int and will revisit the signed/unsigned issue in another diff.

lldb/source/Core/IOHandlerCursesGUI.cpp

public:
 
   int GetCursorX() const { return getcurx(m_window); }
   int GetCursorY() const { return getcury(m_window); }
   void MoveCursor(int x, int y) { ::wmove(m_window, y, x); }
 
   void AttributeOn(attr_t attr) { ::wattron(m_window, attr); }
   void AttributeOff(attr_t attr) { ::wattroff(m_window, attr); }
 
-  int GetMaxX() const { return getmaxx(m_window); }
-  int GetMaxY() const { return getmaxy(m_window); }
-  int GetWidth() const { return GetMaxX(); }
-  int GetHeight() const { return GetMaxY(); }
+  uint32_t GetMaxX() const { return getmaxx(m_window); }
+  uint32_t GetMaxY() const { return getmaxy(m_window); }
+  uint32_t GetWidth() const { return GetMaxX(); }
+  uint32_t GetHeight() const { return GetMaxY(); }
   Size GetSize() const { return Size(GetWidth(), GetHeight()); }
   // Get a zero origin rectangle width the surface size.
   Rect GetFrame() const { return Rect(Point(), GetSize()); }
 
   void Clear() { ::wclear(m_window); }
   void Erase() { ::werase(m_window); }
 
   void SetBackground(int color_pair_idx) {
void DrawError(Surface &surface) {
     surface.MoveCursor(0, 0);
     surface.AttributeOn(COLOR_PAIR(RedOnBlack));
     surface.PutChar(ACS_DIAMOND);
     surface.PutChar(' ');
     surface.PutCStringTruncated(1, m_delegate_sp->GetError().c_str());
     surface.AttributeOff(COLOR_PAIR(RedOnBlack));
 
     surface.MoveCursor(0, 1);
     surface.HorizontalLine(surface.GetWidth());
   }
 
   void DrawFields(Surface &surface) {
     int line = 0;
     int width = surface.GetWidth();
     bool a_field_is_selected = m_selection_type == SelectionType::Field;
     for (int i = 0; i < m_delegate_sp->GetNumberOfFields(); i++) {
       FieldDelegate *field = m_delegate_sp->GetField(i);
public:
   void GetStandardIO(ProcessLaunchInfo &launch_info) {
     if (m_disable_standard_io_field->GetBoolean()) {
       launch_info.GetFlags().Set(eLaunchFlagDisableSTDIO);
       return;
     }
 
     FileAction action;
     if (m_standard_input_field->IsSpecified()) {
-      action.Open(STDIN_FILENO, m_standard_input_field->GetFileSpec(), true,
-                  false);
+      if (action.Open(STDIN_FILENO, m_standard_input_field->GetFileSpec(), true,
+                      false))
         launch_info.AppendFileAction(action);
     }
     if (m_standard_output_field->IsSpecified()) {
-      action.Open(STDOUT_FILENO, m_standard_output_field->GetFileSpec(), false,
-                  true);
+      if (action.Open(STDOUT_FILENO, m_standard_output_field->GetFileSpec(),
+                      false, true))
         launch_info.AppendFileAction(action);
     }
     if (m_standard_error_field->IsSpecified()) {
-      action.Open(STDERR_FILENO, m_standard_error_field->GetFileSpec(), false,
-                  true);
+      if (action.Open(STDERR_FILENO, m_standard_error_field->GetFileSpec(),
+                      false, true))
         launch_info.AppendFileAction(action);
     }
   }
 
   void GetInheritTCC(ProcessLaunchInfo &launch_info) {
     if (m_debugger.GetSelectedTarget()->GetInheritTCC())
       launch_info.GetFlags().Set(eLaunchFlagInheritTCCFromParent);
   }
 
bool WindowDelegateDraw(Window &window, bool force) override {
     m_max_x = window.GetMaxX() - 1;
     m_max_y = window.GetMaxY() - 1;
 
     const uint32_t num_visible_lines = NumVisibleLines();
     StackFrameSP frame_sp;
     bool set_selected_line_to_pc = false;
 
     if (update_location) {
-      const bool process_alive = process ? process->IsAlive() : false;
+      const bool process_alive = process->IsAlive();
       bool thread_changed = false;
       if (process_alive) {
         thread = exe_ctx.GetThreadPtr();
         if (thread) {
           frame_sp = thread->GetSelectedFrame();
           auto tid = thread->GetID();
           thread_changed = tid != m_tid;
           m_tid = tid;
if (num_source_lines > 0) {
               const char *stop_description = stop_info_sp->GetDescription();
               if (stop_description && stop_description[0]) {
                 size_t stop_description_len = strlen(stop_description);
                 int desc_x = window_width - stop_description_len - 16;
                 if (desc_x - window.GetCursorX() > 0)
                   window.Printf("%*s", desc_x - window.GetCursorX(), "");
                 window.MoveCursor(window_width - stop_description_len - 15,
                                   line_y);
+                if (thread)
                   window.PrintfTruncated(1, "<<< Thread %u: %s ",
-                                       thread->GetIndexID(), stop_description);
+                                         thread->GetIndexID(),
+                                         stop_description);
               }
             } else {
               window.Printf("%*s", window_width - window.GetCursorX() - 1, "");
             }
           }
           if (highlight_attr)
             window.AttributeOff(highlight_attr);
         }