Message ID | AN9emJidn3L8OTU1aYJ6d8Ki-naNJPnT4XdNswvhT6CWRfcS0EXEX_n0ULitwr7nPBQAeyAQTZraRsKGnVSbs-kjltjliRAxRejo5ZIaOhE=@gdcproject.org |
---|---|
State | New |
Headers | show |
Hi Iain, hi Pedro, Is this message the latest one on this subject? Christian helped us identify this issue as a regression, and I am wondering whether we want to try to fix it before the 9.1 release or whether we accept it... Thanks! On Tue, Nov 26, 2019 at 11:00:21PM +0000, Iain Buclaw wrote: > ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ > On Tuesday, 26 November 2019 21:24, Pedro Alves <palves@redhat.com> wrote: > > > On 11/26/19 12:49 PM, Iain Buclaw wrote: > > > > > The significance of this is that printf_unfiltered writes messages to wrap_buffer, whereas puts_unfiltered pushes them immediately to stdout, resulting in "post-" messages being printed out of order. > > > > It sounds quite surprising that two _unfiltered functions could behave differently > > like that. That sounds like a bug that should be fixed, instead of worked around > > by having to recall to use printf vs puts. > > > > Thanks, > > Pedro Alves > > I think the best way to avoid the discrepancy is to treat both fputs_filtered and fputs_unfiltered equally by forwarding both calls to fputs_maybe_filtered. > > To avoid recursion, flush_wrap_buffer and fputs_maybe_filtered have had calls to fputs_unfiltered replaced with stream->puts(). > > While attempting to grok my head around fputs_maybe_filtered, I also noticed that buffer_clearer is being removed by the compiler as dead code. > > -- > Iain > > --- > gdb/ChangeLog: > > 2019-11-26 Iain Buclaw <ibuclaw@gdcproject.org> > > * gdb/ui-file.c (fputs_unfiltered): Move to utils.c. > * gdb/utils.c (flush_wrap_buffer): Call ui_file::puts instead of > fputs_unfiltered. > (fputs_maybe_filtered): Likewise. Remove unused buffer_clearer. > (fputs_unfiltered): Moved from utils.c; call fputs_maybe_filtered. > > --- > gdb/ui-file.c | 6 ------ > gdb/utils.c | 22 +++++++++------------- > 2 files changed, 9 insertions(+), 19 deletions(-) > > --- > > > diff --git a/gdb/ui-file.c b/gdb/ui-file.c > index 71b74bba19..31664d5d65 100644 > --- a/gdb/ui-file.c > +++ b/gdb/ui-file.c > @@ -149,12 +149,6 @@ ui_file_read (struct ui_file *file, char *buf, long length_buf) > return file->read (buf, length_buf); > } > > -void > -fputs_unfiltered (const char *buf, struct ui_file *file) > -{ > - file->puts (buf); > -} > - > > > string_file::~string_file () > diff --git a/gdb/utils.c b/gdb/utils.c > index f7fae35729..e8cc21c8c4 100644 > --- a/gdb/utils.c > +++ b/gdb/utils.c > @@ -1539,7 +1539,7 @@ flush_wrap_buffer (struct ui_file *stream) > { > if (stream == gdb_stdout && !wrap_buffer.empty ()) > { > - fputs_unfiltered (wrap_buffer.c_str (), stream); > + stream->puts (wrap_buffer.c_str ()); > wrap_buffer.clear (); > } > } > @@ -1688,18 +1688,10 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream, > || top_level_interpreter ()->interp_ui_out ()->is_mi_like_p ()) > { > flush_wrap_buffer (stream); > - fputs_unfiltered (linebuffer, stream); > + stream->puts (linebuffer); > return; > } > > - auto buffer_clearer > - = make_scope_exit ([&] () > - { > - wrap_buffer.clear (); > - wrap_column = 0; > - wrap_indent = ""; > - }); > - > /* Go through and output each character. Show line extension > when this is necessary; prompt user for new page when this is > necessary. */ > @@ -1788,7 +1780,7 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream, > /* Now output indentation and wrapped string. */ > if (wrap_column) > { > - fputs_unfiltered (wrap_indent, stream); > + stream->puts (wrap_indent); > if (stream->can_emit_style_escape ()) > emit_style_escape (save_style, stream); > /* FIXME, this strlen is what prevents wrap_indent from > @@ -1816,8 +1808,6 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream, > lineptr++; > } > } > - > - buffer_clearer.release (); > } > > void > @@ -1826,6 +1816,12 @@ fputs_filtered (const char *linebuffer, struct ui_file *stream) > fputs_maybe_filtered (linebuffer, stream, 1); > } > > +void > +fputs_unfiltered (const char *linebuffer, struct ui_file *stream) > +{ > + fputs_maybe_filtered (linebuffer, stream, 0); > +} > + > /* See utils.h. */ > > void
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Friday, January 17, 2020 6:56 PM, Joel Brobecker <brobecker@adacore.com> wrote: > Hi Iain, hi Pedro, > > Is this message the latest one on this subject? Christian helped us > identify this issue as a regression, and I am wondering whether we want > to try to fix it before the 9.1 release or whether we accept it... > > Thanks! > Hi, As I've already said, there was a follow-up that instead attempted to fix the filtered vs unfiltered inconsistencies that caused the bug. So rather than this one, I'd suggest looking at the other two here. https://sourceware.org/ml/gdb-patches/2019-11/msg01120.html https://sourceware.org/ml/gdb-patches/2019-11/msg01121.html Regards Iain.
diff --git a/gdb/ui-file.c b/gdb/ui-file.c index 71b74bba19..31664d5d65 100644 --- a/gdb/ui-file.c +++ b/gdb/ui-file.c @@ -149,12 +149,6 @@ ui_file_read (struct ui_file *file, char *buf, long length_buf) return file->read (buf, length_buf); } -void -fputs_unfiltered (const char *buf, struct ui_file *file) -{ - file->puts (buf); -} - string_file::~string_file () diff --git a/gdb/utils.c b/gdb/utils.c index f7fae35729..e8cc21c8c4 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -1539,7 +1539,7 @@ flush_wrap_buffer (struct ui_file *stream) { if (stream == gdb_stdout && !wrap_buffer.empty ()) { - fputs_unfiltered (wrap_buffer.c_str (), stream); + stream->puts (wrap_buffer.c_str ()); wrap_buffer.clear (); } } @@ -1688,18 +1688,10 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream, || top_level_interpreter ()->interp_ui_out ()->is_mi_like_p ()) { flush_wrap_buffer (stream); - fputs_unfiltered (linebuffer, stream); + stream->puts (linebuffer); return; } - auto buffer_clearer - = make_scope_exit ([&] () - { - wrap_buffer.clear (); - wrap_column = 0; - wrap_indent = ""; - }); - /* Go through and output each character. Show line extension when this is necessary; prompt user for new page when this is necessary. */ @@ -1788,7 +1780,7 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream, /* Now output indentation and wrapped string. */ if (wrap_column) { - fputs_unfiltered (wrap_indent, stream); + stream->puts (wrap_indent); if (stream->can_emit_style_escape ()) emit_style_escape (save_style, stream); /* FIXME, this strlen is what prevents wrap_indent from @@ -1816,8 +1808,6 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream, lineptr++; } } - - buffer_clearer.release (); } void @@ -1826,6 +1816,12 @@ fputs_filtered (const char *linebuffer, struct ui_file *stream) fputs_maybe_filtered (linebuffer, stream, 1); } +void +fputs_unfiltered (const char *linebuffer, struct ui_file *stream) +{ + fputs_maybe_filtered (linebuffer, stream, 0); +} + /* See utils.h. */ void