From patchwork Thu Jul 18 18:41:24 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 33736 Received: (qmail 30395 invoked by alias); 18 Jul 2019 18:41:29 -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 30344 invoked by uid 89); 18 Jul 2019 18:41:29 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 18 Jul 2019 18:41:27 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5612C30C62AB for ; Thu, 18 Jul 2019 18:41:26 +0000 (UTC) Received: from localhost.localdomain (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id C5BC0600CC for ; Thu, 18 Jul 2019 18:41:25 +0000 (UTC) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH] Fix new-ui tty stream leak Date: Thu, 18 Jul 2019 19:41:24 +0100 Message-Id: <20190718184124.21907-1-palves@redhat.com> I noticed that we never close the stream opened by new-ui. This fixes it, by adding a new struct ui ctor that takes ownership of the passed-in stream. gdb/ChangeLog: yyyy-mm-dd Pedro Alves * top.c (ui::ui (FILE*, FILE*, FILE*)): Use ui::init. (ui::ui(gdb_file_up&&)): New. (ui::init): New, factored out from 'ui::ui(FILE*, FILE*, FILE*)'. (ui::~ui): Close streams, if they're owned. (new_ui_command): Use ui::ui(gdb_file_up&&)) ctor. * top.h: Include "gdbsupport/filestuff.h". (struct ui): In-class-initialize some fields. (ui::ui(gdb_file_up&&)): Declare. (ui::init): Declare. --- gdb/top.c | 59 ++++++++++++++++++++++++++++++++++++----------------------- gdb/top.h | 36 ++++++++++++++++++++++++++---------- 2 files changed, 62 insertions(+), 33 deletions(-) diff --git a/gdb/top.c b/gdb/top.c index 60f81b3bf85..4e102777409 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -260,27 +260,38 @@ static int highest_ui_num; /* See top.h. */ ui::ui (FILE *instream_, FILE *outstream_, FILE *errstream_) - : next (nullptr), - num (++highest_ui_num), - call_readline (nullptr), - input_handler (nullptr), - command_editing (0), - interp_info (nullptr), - async (0), - secondary_prompt_depth (0), - stdin_stream (instream_), - instream (instream_), + : instream (instream_), outstream (outstream_), errstream (errstream_), - input_fd (fileno (instream)), - input_interactive_p (ISATTY (instream)), - prompt_state (PROMPT_NEEDED), - m_gdb_stdout (new stdio_file (outstream)), - m_gdb_stdin (new stdio_file (instream)), - m_gdb_stderr (new stderr_file (errstream)), - m_gdb_stdlog (m_gdb_stderr), - m_current_uiout (nullptr) + m_owns_streams (false) { + init (); +} + +ui::ui (gdb_file_up &&tty) + : instream (tty.get ()), + outstream (tty.get ()), + errstream (tty.get ()), + m_owns_streams (true) +{ + tty.release (); + + init (); +} + +void +ui::init () +{ + num = ++highest_ui_num; + stdin_stream = instream; + input_fd = fileno (instream); + input_interactive_p = ISATTY (instream); + + m_gdb_stdout = new stdio_file (outstream); + m_gdb_stdin = new stdio_file (instream); + m_gdb_stderr = new stderr_file (errstream); + m_gdb_stdlog = m_gdb_stderr; + buffer_init (&line_buffer); if (ui_list == NULL) @@ -315,6 +326,12 @@ ui::~ui () delete m_gdb_stdin; delete m_gdb_stdout; delete m_gdb_stderr; + + if (m_owns_streams) + { + gdb_assert (instream == outstream && outstream == errstream); + fclose (instream); + } } /* Open file named NAME for read/write, making sure not to make it the @@ -360,8 +377,7 @@ new_ui_command (const char *args, int from_tty) with Windows named pipes. */ gdb_file_up stream = open_terminal_stream (tty_name); - std::unique_ptr ui - (new struct ui (stream.get (), stream.get (), stream.get ())); + std::unique_ptr ui (new struct ui (std::move (stream))); ui->async = 1; @@ -371,9 +387,6 @@ new_ui_command (const char *args, int from_tty) interp_pre_command_loop (top_level_interpreter ()); - /* Make sure the file is not closed. */ - stream.release (); - ui.release (); } diff --git a/gdb/top.h b/gdb/top.h index 32a898b82f5..cef8f851eda 100644 --- a/gdb/top.h +++ b/gdb/top.h @@ -21,6 +21,7 @@ #define TOP_H #include "gdbsupport/buffer.h" +#include "gdbsupport/filestuff.h" #include "event-loop.h" #include "value.h" @@ -55,14 +56,20 @@ enum prompt_state struct ui { - /* Create a new UI. */ + /* Create a new UI. Does not take ownership of the + files/streams. */ ui (FILE *instream, FILE *outstream, FILE *errstream); + + /* Create a new UI, taking ownership of TTY and using it for all of + stdin/stdout/stderr. */ + explicit ui (gdb_file_up &&tty); + ~ui (); DISABLE_COPY_AND_ASSIGN (ui); /* Pointer to next in singly-linked list. */ - struct ui *next; + struct ui *next = nullptr; /* Convenient handle (UI number). Unique across all UIs. */ int num; @@ -77,19 +84,19 @@ struct ui point of invocation. In the special case in which the character read is newline, the function invokes the INPUT_HANDLER callback (see below). */ - void (*call_readline) (gdb_client_data); + void (*call_readline) (gdb_client_data) = nullptr; /* The function to invoke when a complete line of input is ready for processing. */ - void (*input_handler) (gdb::unique_xmalloc_ptr &&); + void (*input_handler) (gdb::unique_xmalloc_ptr &&) = nullptr; /* True if this UI is using the readline library for command editing; false if using GDB's own simple readline emulation, with no editing support. */ - int command_editing; + int command_editing = 0; /* Each UI has its own independent set of interpreters. */ - struct ui_interp_info *interp_info; + struct ui_interp_info *interp_info = nullptr; /* True if the UI is in async mode, false if in sync mode. If in sync mode, a synchronous execution command (e.g, "next") does not @@ -99,11 +106,11 @@ struct ui the top event loop. For the main UI, this starts out disabled, until all the explicit command line arguments (e.g., `gdb -ex "start" -ex "next"') are processed. */ - int async; + int async = 0; /* The number of nested readline secondary prompts that are currently active. */ - int secondary_prompt_depth; + int secondary_prompt_depth = 0; /* The UI's stdin. Set to stdin for the main UI. */ FILE *stdin_stream; @@ -128,7 +135,7 @@ struct ui int input_interactive_p; /* See enum prompt_state's description. */ - enum prompt_state prompt_state; + enum prompt_state prompt_state = PROMPT_NEEDED; /* The fields below that start with "m_" are "private". They're meant to be accessed through wrapper macros that make them look @@ -148,7 +155,16 @@ struct ui struct ui_file *m_gdb_stdlog; /* The current ui_out. */ - struct ui_out *m_current_uiout; + struct ui_out *m_current_uiout = nullptr; + +private: + /* Shared initialization, called by all constructors. */ + void init (); + + /* True if INSTREAM/OUTSTREAM/ERRSTREAM must be closed on + destruction. If true, assumes INSTREAM/OUTSTREAM/ERRSTREAM all + point to the same file. */ + bool m_owns_streams; }; /* The main UI. This is the UI that is bound to stdin/stdout/stderr.