]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commit
gdb: remove final m_stream->emit_style_escape calls from pager_file
authorAndrew Burgess <aburgess@redhat.com>
Tue, 17 Jun 2025 16:39:36 +0000 (17:39 +0100)
committerAndrew Burgess <aburgess@redhat.com>
Wed, 25 Jun 2025 10:43:46 +0000 (11:43 +0100)
commit125881849ad75f05d6c35fdb02a290cb740a75d4
tree2a9d2a1212ab08ed565998c2ef2fb5c640fdda07
parente7b7270ace7c64bbd252d9a152ae541fc28b734f
gdb: remove final m_stream->emit_style_escape calls from pager_file

After the last commit there were still a couple of calls to
m_stream->emit_style_escape in the pager_file class.  As discussed in
the last commit, these are likely wrong, but I'd not been able to
produce any bugs because of them.

The reason why there are no bugs is that these calls are, I think,
entirely redundant.  Consider this block:

      if (m_wrap_column)
{
  /* We are about to insert a newline at an historic
     location in the WRAP_BUFFER.  Before we do we want to
     restore the default style.  To know if we actually
     need to insert an escape sequence we must restore the
     current applied style to how it was at the WRAP_COLUMN
     location.  */
  m_applied_style = m_wrap_style;
  m_stream->emit_style_escape (ui_file_style ());
  /* If we aren't actually wrapping, don't output
     newline -- if chars_per_line is right, we
     probably just overflowed anyway; if it's wrong,
     let us keep going.  */
  m_stream->puts ("\n");
}

What we know (see previous commit) is that the call:

  m_stream->emit_style_escape (ui_file_style ());

is dangerous as m_stream->m_applied_style is going to be out of sync
with its current state.  Actually, m_stream->m_applied_style is likely
to be the default style as it is not updated elsewhere.  So why does
this not cause problems?

Well, GDB's style output is always done in tightly scoped regions.
That means if we want to print some styled output, and then apply a
wrap point the code might look like this:

  fprintf_styled (gdb_stdout, file_name_style, "some text");
  gdb_stdout->wrap_here (4);

But, after printing 'some text', the style of gdb_stdout will have
returned to the default style.

My claim is that, whenever we encounter a wrap_here call, the stream
in question will _always_ have been returned to the default style.

This means that, in the block above, the call:

  m_stream->emit_style_escape (ui_file_style ());

will never emit anything because it depends on a check against
m_stream->m_applied_style, which will always mean that the above call
does nothing.  But that's OK.  By chance, we'll have always placed the
stream into a default style state anyway, so no harm done.

Similarly, the other call:

  /* Having finished inserting the wrapping we should
     restore the style as it was at the WRAP_COLUMN.  */
  m_stream->emit_style_escape (m_wrap_style);

Tries to return m_stream to the state it was in at the point of the
wrap_here call.  But, as described above, this will always be the
default style, so the above call will do nothing, but that just
happens to be exactly what we want!

So what does this commit do?

Well, I "fix" the above code by removing the
m_stream->emit_style_escape calls and replacing them with calls to
puts, passing in the escape sequence for the required style, but only
if the m_stream style as tracked by pager_file::m_stream_style
indicates this is needed.

Got the reasons given above, this should mean there is no change after
this patch.  We still shouldn't be emitting any extra escape
sequences.  But, should we ever manage to get into a state where we
call wrap_here with a stream in a style other than the default, then
this should mean things work as expected.

There should be no user visible changes after this commit.

Approved-By: Tom Tromey <tom@tromey.com>
gdb/pager.h
gdb/utils.c