From f3a09c800fcd1d597fa2b9578cb59adfc15b698d Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Fri, 17 May 2019 14:07:04 +0100 Subject: [PATCH] Change file close behavior for tee_file Instead of using two bools to decide if the files should close when tee_file is closed, make file one stay open and file two close. This simplifies the use cases for it. Inline the make_logging_output into the calling functions (the logic here looks ugly in order to simplify a later change). Expand ui-redirect.exp to cover the changes, similar to mi-logging.exp. gdb/ChangeLog: * cli/cli-interp.c (cli_interp_base::set_logging): Create tee_file directly. * cli/cli-interp.h (make_logging_output): Remove declaration. * cli/cli-logging.c (make_logging_output): Remove function. * mi/mi-interp.c (mi_interp::set_logging): Create tee_file directly. * ui-file.c (tee_file::tee_file): Remove bools. (tee_file::~tee_file): Remove deletes. * ui-file.h (tee_file): Remove bools. gdb/testsuite/ChangeLog: * gdb.base/ui-redirect.exp: Test redirection. --- gdb/ChangeLog | 13 +++++++++ gdb/cli/cli-interp.c | 38 +++++++++++++++----------- gdb/cli/cli-interp.h | 13 --------- gdb/cli/cli-logging.c | 18 ------------ gdb/mi/mi-interp.c | 19 +++++++++---- gdb/testsuite/ChangeLog | 4 +++ gdb/testsuite/gdb.base/ui-redirect.exp | 30 ++++++++++++++++---- gdb/ui-file.c | 11 ++------ gdb/ui-file.h | 15 ++++------ 9 files changed, 86 insertions(+), 75 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index c7e3f325214..2810fff012a 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,16 @@ +2019-05-14 Alan Hayward + Tom Tromey + + * cli/cli-interp.c (cli_interp_base::set_logging): Create tee_file + directly. + * cli/cli-interp.h (make_logging_output): Remove declaration. + * cli/cli-logging.c (make_logging_output): Remove function. + * mi/mi-interp.c (mi_interp::set_logging): Create tee_file + directly. + * ui-file.c (tee_file::tee_file): Remove bools. + (tee_file::~tee_file): Remove deletes. + * ui-file.h (tee_file): Remove bools. + 2019-01-28 Jan Vrany * mi/mi-cmds.h (mi_cmd_complete): New function. diff --git a/gdb/cli/cli-interp.c b/gdb/cli/cli-interp.c index fc4b39a9c2a..7876f910806 100644 --- a/gdb/cli/cli-interp.c +++ b/gdb/cli/cli-interp.c @@ -403,7 +403,7 @@ static saved_output_files saved_output; void cli_interp_base::set_logging (ui_file_up logfile, bool logging_redirect) { - if (logfile != NULL) + if (logfile != nullptr) { saved_output.out = gdb_stdout; saved_output.err = gdb_stderr; @@ -411,16 +411,22 @@ cli_interp_base::set_logging (ui_file_up logfile, bool logging_redirect) saved_output.targ = gdb_stdtarg; saved_output.targerr = gdb_stdtargerr; - /* A raw pointer since ownership is transferred to - gdb_stdout. */ - ui_file *output = make_logging_output (gdb_stdout, - std::move (logfile), - logging_redirect); - gdb_stdout = output; - gdb_stdlog = output; - gdb_stderr = output; - gdb_stdtarg = output; - gdb_stdtargerr = output; + /* If logging is being redirected, then grab logfile. */ + ui_file *logfile_p = nullptr; + if (logging_redirect) + logfile_p = logfile.release (); + + /* If logging is not being redirected, then a tee containing both the + logfile and stdout. */ + ui_file *tee = nullptr; + if (!logging_redirect) + tee = new tee_file (gdb_stdout, std::move (logfile)); + + gdb_stdout = logging_redirect ? logfile_p : tee; + gdb_stdlog = logging_redirect ? logfile_p : tee; + gdb_stderr = logging_redirect ? logfile_p : tee; + gdb_stdtarg = logging_redirect ? logfile_p : tee; + gdb_stdtargerr = logging_redirect ? logfile_p : tee; } else { @@ -434,11 +440,11 @@ cli_interp_base::set_logging (ui_file_up logfile, bool logging_redirect) gdb_stdtarg = saved_output.targ; gdb_stdtargerr = saved_output.targerr; - saved_output.out = NULL; - saved_output.err = NULL; - saved_output.log = NULL; - saved_output.targ = NULL; - saved_output.targerr = NULL; + saved_output.out = nullptr; + saved_output.err = nullptr; + saved_output.log = nullptr; + saved_output.targ = nullptr; + saved_output.targerr = nullptr; } } diff --git a/gdb/cli/cli-interp.h b/gdb/cli/cli-interp.h index 77d73a23d04..0c2e73b6b38 100644 --- a/gdb/cli/cli-interp.h +++ b/gdb/cli/cli-interp.h @@ -33,19 +33,6 @@ public: bool supports_command_editing () override; }; -/* Make the output ui_file to use when logging is enabled. - CURR_OUTPUT is the stream where output is currently being sent to - (e.g., gdb_stdout for the CLI, raw output stream for the MI). - LOGFILE is the log file already opened by the caller. - LOGGING_REDIRECT is the value of the "set logging redirect" - setting. If true, the resulting output is the logfile. If false, - the output stream is a tee, with the log file as one of the - outputs. Ownership of LOGFILE is transferred to the returned - output file, which is an owning pointer. */ -extern ui_file *make_logging_output (ui_file *curr_output, - ui_file_up logfile, - bool logging_redirect); - /* The CLI interpreter's set_logging_proc method. Exported so other interpreters can reuse it. */ extern void cli_set_logging (struct interp *interp, diff --git a/gdb/cli/cli-logging.c b/gdb/cli/cli-logging.c index 3a5e14de3c7..670e7e24908 100644 --- a/gdb/cli/cli-logging.c +++ b/gdb/cli/cli-logging.c @@ -88,24 +88,6 @@ pop_output_files (void) current_uiout->redirect (NULL); } -/* See cli-interp.h. */ - -ui_file * -make_logging_output (ui_file *curr_output, ui_file_up logfile, - bool logging_redirect) -{ - if (logging_redirect) - return logfile.release (); - else - { - /* Note that the "tee" takes ownership of the log file. */ - ui_file *out = new tee_file (curr_output, false, - logfile.get (), true); - logfile.release (); - return out; - } -} - /* This is a helper for the `set logging' command. */ static void handle_redirections (int from_tty) diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c index 4568d398d94..6a19bf02476 100644 --- a/gdb/mi/mi-interp.c +++ b/gdb/mi/mi-interp.c @@ -1286,18 +1286,27 @@ mi_interp::set_logging (ui_file_up logfile, bool logging_redirect) if (logfile != NULL) { mi->saved_raw_stdout = mi->raw_stdout; - mi->raw_stdout = make_logging_output (mi->raw_stdout, - std::move (logfile), - logging_redirect); + /* If something is being redirected, then grab logfile. */ + ui_file *logfile_p = nullptr; + if (logging_redirect) + logfile_p = logfile.release (); + + /* If something is not being redirected, then a tee containing both the + logfile and stdout. */ + ui_file *tee = nullptr; + if (!logging_redirect) + tee = new tee_file (mi->raw_stdout, std::move (logfile)); + + mi->raw_stdout = logging_redirect ? logfile_p : tee; } else { delete mi->raw_stdout; mi->raw_stdout = mi->saved_raw_stdout; - mi->saved_raw_stdout = NULL; + mi->saved_raw_stdout = nullptr; } - + mi->out->set_raw (mi->raw_stdout); mi->err->set_raw (mi->raw_stdout); mi->log->set_raw (mi->raw_stdout); diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index bdabbc72985..6d351564998 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,7 @@ +2019-05-17 Alan Hayward + + * gdb.base/ui-redirect.exp: Test redirection. + 2019-01-28 Jan Vrany * gdb.mi/mi-complete.exp: New file. diff --git a/gdb/testsuite/gdb.base/ui-redirect.exp b/gdb/testsuite/gdb.base/ui-redirect.exp index 1ebff790e57..f1d00b939da 100644 --- a/gdb/testsuite/gdb.base/ui-redirect.exp +++ b/gdb/testsuite/gdb.base/ui-redirect.exp @@ -34,8 +34,28 @@ gdb_test_multiple $test $test { } gdb_test_no_output "end" -gdb_test_no_output "set logging file /dev/null" -gdb_test "set logging on" "Copying output to /dev/null\\." -gdb_test "save breakpoints /dev/null" "Saved to file '/dev/null'\\." -gdb_test "set logging off" "Done logging to /dev/null\\." -gdb_test "help" "List of classes of commands:.*" +with_test_prefix "logging" { + gdb_test_no_output "set logging file /dev/null" + gdb_test "set logging on" "Copying output to /dev/null\\." + gdb_test "save breakpoints /dev/null" "Saved to file '/dev/null'\\." + gdb_test "set logging off" "Done logging to /dev/null\\." + gdb_test "help" "List of classes of commands:.*" +} + +with_test_prefix "redirect" { + gdb_test "set logging redirect on" + gdb_test "set logging on" "Redirecting output to /dev/null\\." + gdb_test_no_output "save breakpoints /dev/null" + gdb_test "set logging off" "Done logging to /dev/null\\." + gdb_test "help" "List of classes of commands:.*" +} + +with_test_prefix "redirect while already logging" { + gdb_test_no_output "set logging redirect off" + gdb_test "set logging on" "Copying output to /dev/null\\." + gdb_test "set logging redirect on" \ + ".*warning: Currently logging .*Turn the logging off and on to make the new setting effective.*" + gdb_test "save breakpoints /dev/null" "Saved to file '/dev/null'\\." + gdb_test "set logging off" "Done logging to /dev/null\\." + gdb_test "help" "List of classes of commands:.*" +} diff --git a/gdb/ui-file.c b/gdb/ui-file.c index 4139b5deffc..24c914f442a 100644 --- a/gdb/ui-file.c +++ b/gdb/ui-file.c @@ -336,20 +336,13 @@ stderr_file::stderr_file (FILE *stream) -tee_file::tee_file (ui_file *one, bool close_one, - ui_file *two, bool close_two) +tee_file::tee_file (ui_file *one, ui_file_up &&two) : m_one (one), - m_two (two), - m_close_one (close_one), - m_close_two (close_two) + m_two (std::move (two)) {} tee_file::~tee_file () { - if (m_close_one) - delete m_one; - if (m_close_two) - delete m_two; } void diff --git a/gdb/ui-file.h b/gdb/ui-file.h index 56f0c0f957c..39f56d5ea42 100644 --- a/gdb/ui-file.h +++ b/gdb/ui-file.h @@ -267,11 +267,9 @@ public: class tee_file : public ui_file { public: - /* Create a file which writes to both ONE and TWO. CLOSE_ONE and - CLOSE_TWO indicate whether the original files should be closed - when the new file is closed. */ - tee_file (ui_file *one, bool close_one, - ui_file *two, bool close_two); + /* Create a file which writes to both ONE and TWO. ONE will remain + open when this object is destroyed; but TWO will be closed. */ + tee_file (ui_file *one, ui_file_up &&two); ~tee_file () override; void write (const char *buf, long length_buf) override; @@ -284,10 +282,9 @@ public: void flush () override; private: - /* The two underlying ui_files, and whether they should each be - closed on destruction. */ - ui_file *m_one, *m_two; - bool m_close_one, m_close_two; + /* The two underlying ui_files. */ + ui_file *m_one; + ui_file_up m_two; }; #endif -- 2.30.2