gdb: Use puts_unfiltered instead of printf_unfiltered

  ‐‐‐‐‐‐‐ 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(-)

---
  

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.