Do not emit style escape sequences to log file
Commit Message
PR gdb/24502 requests that the "set logging" log file not contain
style escape sequences emitted by gdb.
This seemed like a reasonable request to me, so this patch implements
filtering for the log file.
This also updates a comment in ui-style.h that I noticed while writing
the patch.
Tested on x86-64 Fedora 29.
gdb/ChangeLog
2019-06-06 Tom Tromey <tromey@adacore.com>
PR gdb/24502:
* ui-style.h (skip_ansi_escape): Update comment.
* ui-file.h (class no_terminal_escape_file): New class.
* ui-file.c (no_terminal_escape_file::write)
(no_terminal_escape_file::puts): New methods.
* cli/cli-logging.c (handle_redirections): Use
no_terminal_escape_file.
gdb/testsuite/ChangeLog
2019-06-06 Tom Tromey <tromey@adacore.com>
PR gdb/24502:
* gdb.base/style-logging.exp: New file.
---
gdb/ChangeLog | 10 ++++
gdb/cli/cli-logging.c | 2 +-
gdb/testsuite/ChangeLog | 5 ++
gdb/testsuite/gdb.base/style-logging.exp | 70 ++++++++++++++++++++++++
gdb/ui-file.c | 32 +++++++++++
gdb/ui-file.h | 16 ++++++
gdb/ui-style.h | 4 +-
7 files changed, 136 insertions(+), 3 deletions(-)
create mode 100644 gdb/testsuite/gdb.base/style-logging.exp
Comments
On Thursday, June 06 2019, Tom Tromey wrote:
> PR gdb/24502 requests that the "set logging" log file not contain
> style escape sequences emitted by gdb.
>
> This seemed like a reasonable request to me, so this patch implements
> filtering for the log file.
>
> This also updates a comment in ui-style.h that I noticed while writing
> the patch.
>
> Tested on x86-64 Fedora 29.
Thanks for the patch.
I looked at the code, tried my best to understand what it does
(specially how the "skip_ansi_escape" function interacts with the new
"no_terminal_escape_file::puts" method), and everything looks good to
me.
Thanks.
> gdb/ChangeLog
> 2019-06-06 Tom Tromey <tromey@adacore.com>
>
> PR gdb/24502:
> * ui-style.h (skip_ansi_escape): Update comment.
> * ui-file.h (class no_terminal_escape_file): New class.
> * ui-file.c (no_terminal_escape_file::write)
> (no_terminal_escape_file::puts): New methods.
> * cli/cli-logging.c (handle_redirections): Use
> no_terminal_escape_file.
>
> gdb/testsuite/ChangeLog
> 2019-06-06 Tom Tromey <tromey@adacore.com>
>
> PR gdb/24502:
> * gdb.base/style-logging.exp: New file.
> ---
> gdb/ChangeLog | 10 ++++
> gdb/cli/cli-logging.c | 2 +-
> gdb/testsuite/ChangeLog | 5 ++
> gdb/testsuite/gdb.base/style-logging.exp | 70 ++++++++++++++++++++++++
> gdb/ui-file.c | 32 +++++++++++
> gdb/ui-file.h | 16 ++++++
> gdb/ui-style.h | 4 +-
> 7 files changed, 136 insertions(+), 3 deletions(-)
> create mode 100644 gdb/testsuite/gdb.base/style-logging.exp
>
> diff --git a/gdb/cli/cli-logging.c b/gdb/cli/cli-logging.c
> index bef5f3939bd..a8b9955c570 100644
> --- a/gdb/cli/cli-logging.c
> +++ b/gdb/cli/cli-logging.c
> @@ -100,7 +100,7 @@ handle_redirections (int from_tty)
> return;
> }
>
> - stdio_file_up log (new stdio_file ());
> + stdio_file_up log (new no_terminal_escape_file ());
> if (!log->open (logging_filename, logging_overwrite ? "w" : "a"))
> perror_with_name (_("set logging"));
>
> diff --git a/gdb/testsuite/gdb.base/style-logging.exp b/gdb/testsuite/gdb.base/style-logging.exp
> new file mode 100644
> index 00000000000..1d7c3052c38
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/style-logging.exp
> @@ -0,0 +1,70 @@
> +# Copyright 2019 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +# Test that logging does not style.
> +
> +# Do not run if gdb debug is enabled as it will interfere with log redirect.
> +if {[gdb_debug_enabled]} {
> + untested "debug is enabled"
> + return 0
> +}
> +
> +if {[is_remote host]} {
> + untested "does not work on remote host"
> + return 0
> +}
> +
> +standard_testfile style.c
> +
> +save_vars { env(TERM) } {
> + # We need an ANSI-capable terminal to get the output.
> + setenv TERM ansi
> +
> + if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
> + return -1
> + }
> +
> + if {![runto_main]} {
> + fail "style tests failed"
> + return
> + }
> +
> + gdb_test_no_output "set style enabled on"
> +
> + set log_name [standard_output_file log.txt]
> + gdb_test_no_output "set logging file $log_name"
> + gdb_test_no_output "set logging overwrite on"
> + gdb_test "set logging on" "Copying output to .*"
> +
> + set main_expr [style main function]
> + set base_file_expr [style ".*style\\.c" file]
> + set file_expr "$base_file_expr:\[0-9\]"
> + set arg_expr [style "arg." variable]
> + gdb_test "frame" \
> + "$main_expr.*$arg_expr.*$arg_expr.*$file_expr.*"
> +
> + gdb_test "set logging off" "Done logging to .*"
> +
> + set fd [open $log_name]
> + set data [read -nonewline $fd]
> + close $fd
> +
> + set testname "log is escape-free"
> + if {[regexp "\033" $data]} {
> + fail $testname
> + } else {
> + pass $testname
> + }
> +}
> diff --git a/gdb/ui-file.c b/gdb/ui-file.c
> index 24c914f442a..05d411aa392 100644
> --- a/gdb/ui-file.c
> +++ b/gdb/ui-file.c
> @@ -396,3 +396,35 @@ tee_file::can_emit_style_escape ()
> && m_one->term_out ()
> && term_cli_styling ());
> }
> +
> +/* See ui-file.h. */
> +
> +void
> +no_terminal_escape_file::write (const char *buf, long length_buf)
> +{
> + std::string copy (buf, length_buf);
> + this->puts (copy.c_str ());
> +}
> +
> +/* See ui-file.h. */
> +
> +void
> +no_terminal_escape_file::puts (const char *buf)
> +{
> + while (*buf != '\0')
> + {
> + const char *esc = strchr (buf, '\033');
> + if (esc == nullptr)
> + break;
> +
> + int n_read = 0;
> + if (!skip_ansi_escape (esc, &n_read))
> + ++esc;
> +
> + this->stdio_file::write (buf, esc - buf);
> + buf = esc + n_read;
> + }
> +
> + if (*buf != '\0')
> + this->stdio_file::write (buf, strlen (buf));
> +}
> diff --git a/gdb/ui-file.h b/gdb/ui-file.h
> index 39f56d5ea42..3f6f38a68fb 100644
> --- a/gdb/ui-file.h
> +++ b/gdb/ui-file.h
> @@ -287,4 +287,20 @@ private:
> ui_file_up m_two;
> };
>
> +/* A ui_file implementation that filters out terminal escape
> + sequences. */
> +
> +class no_terminal_escape_file : public stdio_file
> +{
> +public:
> + no_terminal_escape_file ()
> + {
> + }
> +
> + /* Like the stdio_file methods, but these filter out terminal escape
> + sequences. */
> + void write (const char *buf, long length_buf) override;
> + void puts (const char *linebuffer) override;
> +};
> +
> #endif
> diff --git a/gdb/ui-style.h b/gdb/ui-style.h
> index 2a87fbe8012..24b4b59ed9f 100644
> --- a/gdb/ui-style.h
> +++ b/gdb/ui-style.h
> @@ -233,8 +233,8 @@ private:
>
> /* Skip an ANSI escape sequence in BUF. BUF must begin with an ESC
> character. Return true if an escape sequence was successfully
> - skipped; false otherwise. In either case, N_READ is updated to
> - reflect the number of chars read from BUF. */
> + skipped; false otherwise. If an escape sequence was skipped,
> + N_READ is updated to reflect the number of chars read from BUF. */
>
> extern bool skip_ansi_escape (const char *buf, int *n_read);
>
> --
> 2.20.1
>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@redhat.com> writes:
>> PR gdb/24502 requests that the "set logging" log file not contain
>> style escape sequences emitted by gdb.
>>
>> This seemed like a reasonable request to me, so this patch implements
>> filtering for the log file.
>>
>> This also updates a comment in ui-style.h that I noticed while writing
>> the patch.
>>
>> Tested on x86-64 Fedora 29.
Sergio> Thanks for the patch.
Sergio> I looked at the code, tried my best to understand what it does
Sergio> (specially how the "skip_ansi_escape" function interacts with the new
Sergio> "no_terminal_escape_file::puts" method), and everything looks good to
Sergio> me.
Thanks for taking a look.
I'm checking this in now.
Tom
@@ -100,7 +100,7 @@ handle_redirections (int from_tty)
return;
}
- stdio_file_up log (new stdio_file ());
+ stdio_file_up log (new no_terminal_escape_file ());
if (!log->open (logging_filename, logging_overwrite ? "w" : "a"))
perror_with_name (_("set logging"));
new file mode 100644
@@ -0,0 +1,70 @@
+# Copyright 2019 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test that logging does not style.
+
+# Do not run if gdb debug is enabled as it will interfere with log redirect.
+if {[gdb_debug_enabled]} {
+ untested "debug is enabled"
+ return 0
+}
+
+if {[is_remote host]} {
+ untested "does not work on remote host"
+ return 0
+}
+
+standard_testfile style.c
+
+save_vars { env(TERM) } {
+ # We need an ANSI-capable terminal to get the output.
+ setenv TERM ansi
+
+ if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+ return -1
+ }
+
+ if {![runto_main]} {
+ fail "style tests failed"
+ return
+ }
+
+ gdb_test_no_output "set style enabled on"
+
+ set log_name [standard_output_file log.txt]
+ gdb_test_no_output "set logging file $log_name"
+ gdb_test_no_output "set logging overwrite on"
+ gdb_test "set logging on" "Copying output to .*"
+
+ set main_expr [style main function]
+ set base_file_expr [style ".*style\\.c" file]
+ set file_expr "$base_file_expr:\[0-9\]"
+ set arg_expr [style "arg." variable]
+ gdb_test "frame" \
+ "$main_expr.*$arg_expr.*$arg_expr.*$file_expr.*"
+
+ gdb_test "set logging off" "Done logging to .*"
+
+ set fd [open $log_name]
+ set data [read -nonewline $fd]
+ close $fd
+
+ set testname "log is escape-free"
+ if {[regexp "\033" $data]} {
+ fail $testname
+ } else {
+ pass $testname
+ }
+}
@@ -396,3 +396,35 @@ tee_file::can_emit_style_escape ()
&& m_one->term_out ()
&& term_cli_styling ());
}
+
+/* See ui-file.h. */
+
+void
+no_terminal_escape_file::write (const char *buf, long length_buf)
+{
+ std::string copy (buf, length_buf);
+ this->puts (copy.c_str ());
+}
+
+/* See ui-file.h. */
+
+void
+no_terminal_escape_file::puts (const char *buf)
+{
+ while (*buf != '\0')
+ {
+ const char *esc = strchr (buf, '\033');
+ if (esc == nullptr)
+ break;
+
+ int n_read = 0;
+ if (!skip_ansi_escape (esc, &n_read))
+ ++esc;
+
+ this->stdio_file::write (buf, esc - buf);
+ buf = esc + n_read;
+ }
+
+ if (*buf != '\0')
+ this->stdio_file::write (buf, strlen (buf));
+}
@@ -287,4 +287,20 @@ private:
ui_file_up m_two;
};
+/* A ui_file implementation that filters out terminal escape
+ sequences. */
+
+class no_terminal_escape_file : public stdio_file
+{
+public:
+ no_terminal_escape_file ()
+ {
+ }
+
+ /* Like the stdio_file methods, but these filter out terminal escape
+ sequences. */
+ void write (const char *buf, long length_buf) override;
+ void puts (const char *linebuffer) override;
+};
+
#endif
@@ -233,8 +233,8 @@ private:
/* Skip an ANSI escape sequence in BUF. BUF must begin with an ESC
character. Return true if an escape sequence was successfully
- skipped; false otherwise. In either case, N_READ is updated to
- reflect the number of chars read from BUF. */
+ skipped; false otherwise. If an escape sequence was skipped,
+ N_READ is updated to reflect the number of chars read from BUF. */
extern bool skip_ansi_escape (const char *buf, int *n_read);