Re: [PATCHv2 2/2] gdb/tui: asm window handles invalid memory and scrolls better

[Shahab Vahedi <Shahab dot Vahedi at synopsys dot com>]

Great job Andrew!

Functionality testing:
I have tested this patch and it is impossible to break it.

Code review:
The comments that have been added are very self explanatory.
The logic that is used seems sound and thorough. I have one
small remark though. For most part of the
tui_find_disassembly_address() function, there are usages of
new_low, prev_low, and possible_new_low variables. What these
variables are actually used for is finding a new _high_. I
suggest replacing every instance of "low/LOW" with "high/HIGH".
The region of code I am talking about is listed below:

region of code begins
  else
    {
      ...
      gdb::optional<CORE_ADDR> possible_new_low;
      ...
      CORE_ADDR prev_low;

      do
        {
          ...
        }
      while (...)
      ...
      if (asm_lines.size () < max_lines)
        {
          if (!possible_new_low.has_value ())
            return pc;

          new_low = *possible_new_low;
          next_addr = tui_disassemble (gdbarch, asm_lines,
                                       new_low, max_lines);
          ...
        }
region of code ends

Nevertheless, the code looks (very) fine to me as it is.


Cheers,
Shahab