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

[Tom Tromey <tom@tromey.com>]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> Adding tests for this scrolling was a little bit of a problem.  First
Andrew> I would have liked to add tests for PageUp / PageDown, but the tuiterm
Andrew> library we use doesn't support these commands.

I wonder if setting TERM to xterm or vt100 would let this work without
too much effort.

Andrew> Next, I would have liked to test scrolling to the start or end of the
Andrew> assembler listing and then trying to scroll even more [...]

Andrew> The problem is that there is no curses output, so how long do we wait
Andrew> at step 3?

Resizing had the same problem (how to tell when the resize is finished),
so I added a special mode to the TUI for this.  So, if you really
wanted, in this case you could have the TUI debug mode print something
or ring the bell when scrolling isn't possible.

Andrew> +      asm_lines.push_back (tal);

This should probably use push_back (std::move (tal)), to avoid copying
the string.

Andrew> +      /* As we search backward if we find an address that looks promising
Andrew> +	 then we record it in this structure.  If the next address we try
Andrew> +	 is not suitable then we fall back to the previous good address.
Andrew> +	 Otherwise, if the next address is also good it gets recorded here
Andrew> +	 instead, and then we try the next address.  */
Andrew> +      struct
Andrew> +      {
Andrew> +	bool found = false;
Andrew> +	CORE_ADDR new_low;
Andrew> +      } possible_new_low;

This can be gdb::optional<CORE_ADDR>, which would seem clearer in this
case to me.


Aside from these nits, this seems fine to me.  I didn't try it, but if
you can reproduce the problems Shahab saw, I think it would be good to
incorporate his suggested change and also file a TUI bug for the
remaining problem (unless you feel like fixing it as well...)

Thanks for doing this.  This is a tricky area.

Tom