[PATCH] Fix pager regression

[PATCH] Fix pager regression

[Tom Tromey]

The patch to fix paging with redirection caused a regression in the
internal AdaCore test suite.  The problem occurs when running an MI
command from the CLI using interpreter-exec, when paging is enabled.
This scenario isn't covered by the current test suite, so this patch
includes a new test.

The problem is that, in this situation, MI does:

  fputs_unfiltered (strcmp (context->command, "target-select") == 0
		     ? "^connected" : "^done", mi->raw_stdout);

Here raw_stdout is a stdio_file wrapping stdout, so the pager thinks
that it is ok to buffer the output.  However, in this setup, it isn't
ok, and flushing the wrap buffer doesn't really work properly.  Also,
MI next does:

  mi_out_put (uiout, mi->raw_stdout);

... but this uses ui_file::write, which also doesn't flush the wrap
buffer.

I think all this will be fixed by the pager rewrite series I'm working
on.  However, in the meantime, adding the old gdb_stdout check back to
the pager fixes this problem.

Regression tested on x86-64 Fedora 34.
---
 gdb/testsuite/gdb.base/page-logging.exp | 10 ++++++++++
 gdb/utils.c                             |  1 +
 2 files changed, 11 insertions(+)

diff --git a/gdb/testsuite/gdb.base/page-logging.exp b/gdb/testsuite/gdb.base/page-logging.exp
index 05a3f793f5d..ffc180aad4d 100644
--- a/gdb/testsuite/gdb.base/page-logging.exp
+++ b/gdb/testsuite/gdb.base/page-logging.exp
@@ -48,3 +48,13 @@ if {$ok} {
     fail "printf without paging"
 }
 gdb_test "set logging enabled off" "Done logging to .*"
+
+set cmd "interpreter-exec mi2 \"-data-evaluate-expression 23\""
+gdb_test_multiple $cmd $cmd {
+    -re ".done.value=.23\[^\n\]+\r\n$gdb_prompt " {
+	pass "$cmd"
+	gdb_expect 1 {
+	    -re "\r\n$gdb_prompt $" { }
+	}
+    }
+}
diff --git a/gdb/utils.c b/gdb/utils.c
index 11f88d6f991..df4e6c1fd58 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1750,6 +1750,7 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
 
   /* Don't do any filtering if it is disabled.  */
   if (!stream->can_page ()
+      || stream != gdb_stdout
       || !pagination_enabled
       || pagination_disabled_for_command
       || batch_flag
-- 
2.31.1

Re: [PATCH] Fix pager regression

[Andrew Burgess]

* Tom Tromey <tromey@adacore.com> [2022-01-03 09:54:29 -0700]:

> The patch to fix paging with redirection caused a regression in the
> internal AdaCore test suite.  The problem occurs when running an MI
> command from the CLI using interpreter-exec, when paging is enabled.
> This scenario isn't covered by the current test suite, so this patch
> includes a new test.
> 
> The problem is that, in this situation, MI does:
> 
>   fputs_unfiltered (strcmp (context->command, "target-select") == 0
> 		     ? "^connected" : "^done", mi->raw_stdout);
> 
> Here raw_stdout is a stdio_file wrapping stdout, so the pager thinks
> that it is ok to buffer the output.  However, in this setup, it isn't
> ok, and flushing the wrap buffer doesn't really work properly.  Also,
> MI next does:
> 
>   mi_out_put (uiout, mi->raw_stdout);
> 
> ... but this uses ui_file::write, which also doesn't flush the wrap
> buffer.
> 
> I think all this will be fixed by the pager rewrite series I'm working
> on.  However, in the meantime, adding the old gdb_stdout check back to
> the pager fixes this problem.

You explanation makes sense to me.  So LGTM.

Thanks,
Andrew

> 
> Regression tested on x86-64 Fedora 34.
> ---
>  gdb/testsuite/gdb.base/page-logging.exp | 10 ++++++++++
>  gdb/utils.c                             |  1 +
>  2 files changed, 11 insertions(+)
> 
> diff --git a/gdb/testsuite/gdb.base/page-logging.exp b/gdb/testsuite/gdb.base/page-logging.exp
> index 05a3f793f5d..ffc180aad4d 100644
> --- a/gdb/testsuite/gdb.base/page-logging.exp
> +++ b/gdb/testsuite/gdb.base/page-logging.exp
> @@ -48,3 +48,13 @@ if {$ok} {
>      fail "printf without paging"
>  }
>  gdb_test "set logging enabled off" "Done logging to .*"
> +
> +set cmd "interpreter-exec mi2 \"-data-evaluate-expression 23\""
> +gdb_test_multiple $cmd $cmd {
> +    -re ".done.value=.23\[^\n\]+\r\n$gdb_prompt " {
> +	pass "$cmd"
> +	gdb_expect 1 {
> +	    -re "\r\n$gdb_prompt $" { }
> +	}
> +    }
> +}
> diff --git a/gdb/utils.c b/gdb/utils.c
> index 11f88d6f991..df4e6c1fd58 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -1750,6 +1750,7 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
>  
>    /* Don't do any filtering if it is disabled.  */
>    if (!stream->can_page ()
> +      || stream != gdb_stdout
>        || !pagination_enabled
>        || pagination_disabled_for_command
>        || batch_flag
> -- 
> 2.31.1
> 
>