From patchwork Mon Apr 29 12:50:47 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alan Hayward X-Patchwork-Id: 32453 Received: (qmail 5670 invoked by alias); 29 Apr 2019 12:50:58 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 5569 invoked by uid 89); 29 Apr 2019 12:50:57 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.8 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, MIME_BASE64_BLANKS, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS autolearn=ham version=3.3.1 spammy=transferred, tee, Done, mi_interp X-HELO: EUR02-VE1-obe.outbound.protection.outlook.com Received: from mail-eopbgr20041.outbound.protection.outlook.com (HELO EUR02-VE1-obe.outbound.protection.outlook.com) (40.107.2.41) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 29 Apr 2019 12:50:54 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector1-arm-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=xw0DwCpUlDPhOXIN7ulqPssVEd4b68OLG7x8UneeyT4=; b=g9iFvgxPuAhgjyfGhQtorrAyjppWpJe5EtimXGgoRloG1cIxMW3kHpp8SwZqDLgQIUPDieQdnHqvnmH4ne6NKjEpm9NxU8/sNDd6miBSEPAHvVsDtjv9K9gwxubeYM6pNYE5eO72ArKp67hfg6+As8L08AjCeeu9blNo55ugpJQ= Received: from DB6PR0802MB2133.eurprd08.prod.outlook.com (10.172.227.22) by DB6PR0802MB2518.eurprd08.prod.outlook.com (10.172.245.10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1835.15; Mon, 29 Apr 2019 12:50:47 +0000 Received: from DB6PR0802MB2133.eurprd08.prod.outlook.com ([fe80::399b:3a32:bff9:827e]) by DB6PR0802MB2133.eurprd08.prod.outlook.com ([fe80::399b:3a32:bff9:827e%11]) with mapi id 15.20.1835.018; Mon, 29 Apr 2019 12:50:47 +0000 From: Alan Hayward To: "gdb-patches@sourceware.org" CC: nd , Alan Hayward Subject: [PATCH v2 1/4] Change file close behavior for tee_file Date: Mon, 29 Apr 2019 12:50:47 +0000 Message-ID: <20190429125041.15266-2-alan.hayward@arm.com> References: <20190429125041.15266-1-alan.hayward@arm.com> In-Reply-To: <20190429125041.15266-1-alan.hayward@arm.com> authentication-results: spf=none (sender IP is ) smtp.mailfrom=Alan.Hayward@arm.com; x-ms-oob-tlc-oobclassifiers: OLM:7691; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 MIME-Version: 1.0 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-IsSubscribed: yes 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 cover the changes, similar to how mi-logging.exp already works. gdb/ChangeLog: 2019-04-29 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. gdb/testsuite/ChangeLog: 2019-04-29 Alan Hayward * gdb.base/ui-redirect.exp: Test redirection. --- 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/gdb.base/ui-redirect.exp | 30 ++++++++++++++++---- gdb/ui-file.c | 11 ++------ gdb/ui-file.h | 15 ++++------ 7 files changed, 69 insertions(+), 75 deletions(-) -- 2.20.1 (Apple Git-117) diff --git a/gdb/cli/cli-interp.c b/gdb/cli/cli-interp.c index fc4b39a9c2..7876f91080 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 77d73a23d0..0c2e73b6b3 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 3a5e14de3c..670e7e2490 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 4568d398d9..6a19bf0247 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/gdb.base/ui-redirect.exp b/gdb/testsuite/gdb.base/ui-redirect.exp index 1ebff790e5..f1d00b939d 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 4139b5deff..24c914f442 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 56f0c0f957..39f56d5ea4 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