gdb: Use puts_unfiltered instead of printf_unfiltered

Message ID Bp1z1H0Uk-JcW1j7NMWgeijH4pFLAM5zyY6-OMDHyXwIBLc_qKy0LFyx15VMcg6gEd4IKqxbhN1mBXE5Hd0jY0U9qZj0-Poz4hQHHoYlKDc=@gdcproject.org
State New, archived
Headers

Commit Message

Iain Buclaw Nov. 26, 2019, 12:49 p.m. UTC
  Hi,

This patch fixes a regression, possibly introduced by 2a3c1174c3c0db1140180fb3fc56ac324d1c0a7c, in this part of the change:

---
@@ -2064,13 +2096,13 @@ vfprintf_unfiltered (struct ui_file *stream, const char *format, va_list args)
       fputs_unfiltered (timestamp.c_str (), stream);
     }
   else
-    fputs_unfiltered (linebuffer.c_str (), stream);
+    vfprintf_maybe_filtered (stream, format, args, false, true);
 }

 void
 vprintf_filtered (const char *format, va_list args)
---

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.

Not sure about how to go about testing this, looking at the testsuite, such as gdb.base/annota1.exp, everything appears to be in order.  Perhaps this is because the testsuite triggers one of these conditions in fputs_maybe_filtered() though.

---
if (stream != gdb_stdout
    || !pagination_enabled
    || pagination_disabled_for_command
    || batch_flag
    || (lines_per_page == UINT_MAX && chars_per_line == UINT_MAX)
    || top_level_interpreter () == NULL
    || top_level_interpreter ()->interp_ui_out ()->is_mi_like_p ())
---


However, the actual observed behaviour in gdb is:
---
Reading symbols from a.out...
(gdb) set annotate 2

\032\032pre-prompt
(gdb)
\032\032prompt
start

prompt\032\032post-
Temporary breakpoint 1 at 0x13716: file test.c, line 3.
---

With this patch applied, instead "\032\032post-prompt" is printed.

I think this can be applied as obvious, but wanted to have someone else have a quick check, just in case it would be preferred to change fputs_maybe_filtered instead to flush the buffer on scope exit for unfiltered messages.

--
Iain

---
  

Comments

Terekhov, Mikhail via Gdb-patches Nov. 26, 2019, 8:13 p.m. UTC | #1
Sounds like this fixes
https://sourceware.org/bugzilla/show_bug.cgi?id=25190, could you
mention that in the patch description?

(Those three lines could probably be a single printf...)

Christian

On Tue, Nov 26, 2019 at 6:49 AM Iain Buclaw <ibuclaw@gdcproject.org> wrote:
>
> Hi,
>
> This patch fixes a regression, possibly introduced by 2a3c1174c3c0db1140180fb3fc56ac324d1c0a7c, in this part of the change:
>
> ---
> @@ -2064,13 +2096,13 @@ vfprintf_unfiltered (struct ui_file *stream, const char *format, va_list args)
>        fputs_unfiltered (timestamp.c_str (), stream);
>      }
>    else
> -    fputs_unfiltered (linebuffer.c_str (), stream);
> +    vfprintf_maybe_filtered (stream, format, args, false, true);
>  }
>
>  void
>  vprintf_filtered (const char *format, va_list args)
> ---
>
> 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.
>
> Not sure about how to go about testing this, looking at the testsuite, such as gdb.base/annota1.exp, everything appears to be in order.  Perhaps this is because the testsuite triggers one of these conditions in fputs_maybe_filtered() though.
>
> ---
> if (stream != gdb_stdout
>     || !pagination_enabled
>     || pagination_disabled_for_command
>     || batch_flag
>     || (lines_per_page == UINT_MAX && chars_per_line == UINT_MAX)
>     || top_level_interpreter () == NULL
>     || top_level_interpreter ()->interp_ui_out ()->is_mi_like_p ())
> ---
>
>
> However, the actual observed behaviour in gdb is:
> ---
> Reading symbols from a.out...
> (gdb) set annotate 2
>
> \032\032pre-prompt
> (gdb)
> \032\032prompt
> start
>
> prompt\032\032post-
> Temporary breakpoint 1 at 0x13716: file test.c, line 3.
> ---
>
> With this patch applied, instead "\032\032post-prompt" is printed.
>
> I think this can be applied as obvious, but wanted to have someone else have a quick check, just in case it would be preferred to change fputs_maybe_filtered instead to flush the buffer on scope exit for unfiltered messages.
>
> --
> Iain
>
> ---
>
>
>
  
Pedro Alves Nov. 26, 2019, 8:24 p.m. UTC | #2
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
  

Patch

gdb/ChangeLog:

2019-11-26  Iain Buclaw  <ibuclaw@gdcproject.org>

	* gdb/event-top.c (handle_line_of_input): Use puts_unfiltered instead
	of printf_unfiltered.

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 0396dbcc52..df6a4095fb 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -663,9 +663,9 @@  handle_line_of_input (struct buffer *cmd_line_buffer,
 
   if (from_tty && annotation_level > 1)
     {
-      printf_unfiltered (("\n\032\032post-"));
+      puts_unfiltered ("\n\032\032post-");
       puts_unfiltered (annotation_suffix);
-      printf_unfiltered (("\n"));
+      puts_unfiltered ("\n");
     }
 
 #define SERVER_COMMAND_PREFIX "server "