From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 127760 invoked by alias); 25 Jan 2017 19:31:36 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 127749 invoked by uid 89); 25 Jan 2017 19:31:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.1 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Bah, *uiout, H*i:sk:8bf8c6d, H*f:sk:8bf8c6d 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; Wed, 25 Jan 2017 19:31:33 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7E3CBC04B92A; Wed, 25 Jan 2017 19:31:33 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.phx2.redhat.com [10.5.9.4]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v0PJVT89008389; Wed, 25 Jan 2017 14:31:32 -0500 Subject: Re: [PATCH v2 5/5] Eliminate make_cleanup_ui_file_delete / make ui_file a class hierarchy To: Simon Marchi References: <1484617147-2506-1-git-send-email-palves@redhat.com> <1484617147-2506-6-git-send-email-palves@redhat.com> <5533317a1fdb2e5f36e64731aef84993@polymtl.ca> <4a6cce94-1533-36e9-6015-330e2e014a98@redhat.com> <558e1e55-23eb-716e-8117-c18d7bded7b8@redhat.com> <79c06dbccef0c33309d208316be207f4@polymtl.ca> <8bf8c6d9-9bbb-be9b-ee5e-99c2c3fa5b8e@redhat.com> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: Date: Wed, 25 Jan 2017 19:31:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <8bf8c6d9-9bbb-be9b-ee5e-99c2c3fa5b8e@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-01/txt/msg00553.txt.bz2 On 01/25/2017 05:52 PM, Pedro Alves wrote: > >> > >>> >> + if (!log->open (logging_filename, logging_overwrite ? "w" : "a")) >>> >> perror_with_name (_("set logging")); >>> >> - cleanups = make_cleanup_ui_file_delete (output); >>> >> + >>> >> + output = log.get (); >>> >> >>> >> /* Redirects everything to gdb_stdout while this is running. */ >>> >> if (!logging_redirect) >>> >> { >>> >> no_redirect_file = output; >>> >> >>> >> - output = tee_file_new (gdb_stdout, 0, no_redirect_file, 0); >>> >> - if (output == NULL) >>> >> - perror_with_name (_("set logging")); >>> >> - make_cleanup_ui_file_delete (output); >>> >> + output = new tee_file (gdb_stdout, 0, no_redirect_file, 0); >> > >> > Is there anything that replaces this cleanup? IOW, if the >> > fprinf_unfiltered just below did throw an exception for some reason, >> > would this tee_file leak? > Looks like there isn't. I'll fix it. Bah, this is going to > mess up a follow-up patch to push down the tee to the interpreter > callback that I had prepared meanwhile. :-) > Here's the diff to the incoming v3. I think this addresses all your comments. The only other change is that I realized that ui_file should be abstract, so that people don't try to allocate one to build a "/dev/null" stream. I've checked the AIX build on gcc119. I'll send v3 shortly. The "tee" patch I was mentioning will make current_interp_set_logging take a ui_file_up and a bool instead of two ui_file * pointers, so some of the ".get()s" will end up disappearing (if that goes in). >From 4e444042b03c07f8387c872f9ded53bb81082a90 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Wed, 25 Jan 2017 18:52:31 +0000 Subject: [PATCH] v2-review --- gdb/aix-thread.c | 3 +-- gdb/arm-tdep.c | 11 ++++------- gdb/cli/cli-logging.c | 34 +++++++++++++++++----------------- gdb/mi/mi-console.c | 6 ------ gdb/mi/mi-console.h | 2 -- gdb/serial.c | 2 +- gdb/ui-file.h | 18 ++++++++++++++---- gdb/utils.h | 3 --- 8 files changed, 37 insertions(+), 42 deletions(-) diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c index cc14d9d..cf1a462 100644 --- a/gdb/aix-thread.c +++ b/gdb/aix-thread.c @@ -1749,7 +1749,6 @@ static char * aix_thread_extra_thread_info (struct target_ops *self, struct thread_info *thread) { - struct ui_file *buf; int status; pthdb_pthread_t pdtid; pthdb_tid_t tid; @@ -1797,7 +1796,7 @@ aix_thread_extra_thread_info (struct target_ops *self, xfree (ret); /* Free old buffer. */ - ret = xstrdup (buf.string ()); + ret = xstrdup (buf.c_str ()); return ret; } diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index bc170b1..efce97b 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -9578,7 +9578,6 @@ _initialize_arm_tdep (void) const char *setdesc; const char *const *regnames; int i; - static std::string helptext; char regdesc[1024], *rdptr = regdesc; size_t rest = sizeof (regdesc); @@ -9644,12 +9643,10 @@ _initialize_arm_tdep (void) valid_disassembly_styles[num_disassembly_options] = NULL; /* Create the help text. */ - string_file stb; - stb.printf ("%s%s%s", - _("The valid values are:\n"), - regdesc, - _("The default is \"std\".")); - helptext = std::move (stb.string ()); + std::string helptext = string_printf ("%s%s%s", + _("The valid values are:\n"), + regdesc, + _("The default is \"std\".")); add_setshow_enum_cmd("disassembler", no_class, valid_disassembly_styles, &disassembly_style, diff --git a/gdb/cli/cli-logging.c b/gdb/cli/cli-logging.c index f9a845e..f165896 100644 --- a/gdb/cli/cli-logging.c +++ b/gdb/cli/cli-logging.c @@ -178,8 +178,8 @@ pop_output_files (void) static void handle_redirections (int from_tty) { - struct ui_file *output; - struct ui_file *no_redirect_file = NULL; + ui_file_up output; + ui_file_up no_redirect_file; if (saved_filename != NULL) { @@ -188,34 +188,30 @@ handle_redirections (int from_tty) return; } - std::unique_ptr log (new stdio_file ()); + stdio_file_up log (new stdio_file ()); if (!log->open (logging_filename, logging_overwrite ? "w" : "a")) perror_with_name (_("set logging")); - output = log.get (); - /* Redirects everything to gdb_stdout while this is running. */ if (!logging_redirect) { - no_redirect_file = output; + no_redirect_file = std::move (log); + output.reset (new tee_file (gdb_stdout, 0, no_redirect_file.get (), 0)); - output = new tee_file (gdb_stdout, 0, no_redirect_file, 0); if (from_tty) fprintf_unfiltered (gdb_stdout, "Copying output to %s.\n", logging_filename); - logging_no_redirect_file = no_redirect_file; } else { gdb_assert (logging_no_redirect_file == NULL); + output = std::move (log); if (from_tty) fprintf_unfiltered (gdb_stdout, "Redirecting output to %s.\n", logging_filename); } - log.release (); - saved_filename = xstrdup (logging_filename); saved_output.out = gdb_stdout; saved_output.err = gdb_stderr; @@ -224,18 +220,22 @@ handle_redirections (int from_tty) saved_output.targerr = gdb_stdtargerr; /* Let the interpreter do anything it needs. */ - if (current_interp_set_logging (1, output, no_redirect_file) == 0) + if (current_interp_set_logging (1, output.get (), + no_redirect_file.get ()) == 0) { - gdb_stdout = output; - gdb_stdlog = output; - gdb_stderr = output; - gdb_stdtarg = output; - gdb_stdtargerr = output; + gdb_stdout = output.get (); + gdb_stdlog = output.get (); + gdb_stderr = output.get (); + gdb_stdtarg = output.get (); + gdb_stdtargerr = output.get (); } + output.release (); + logging_no_redirect_file = no_redirect_file.release (); + /* Don't do the redirect for MI, it confuses MI's ui-out scheme. */ if (!current_uiout->is_mi_like_p ()) - current_uiout->redirect (output); + current_uiout->redirect (gdb_stdout); } static void diff --git a/gdb/mi/mi-console.c b/gdb/mi/mi-console.c index 218ffbc..7c1c2ac 100644 --- a/gdb/mi/mi-console.c +++ b/gdb/mi/mi-console.c @@ -37,12 +37,6 @@ mi_console_file::mi_console_file (ui_file *raw, const char *prefix, char quote) {} void -mi_console_file::puts (const char *buf) -{ - this->write (buf, strlen (buf)); -} - -void mi_console_file::write (const char *buf, long length_buf) { size_t prev_size = m_buffer.size (); diff --git a/gdb/mi/mi-console.h b/gdb/mi/mi-console.h index a7962a8..289013f 100644 --- a/gdb/mi/mi-console.h +++ b/gdb/mi/mi-console.h @@ -39,8 +39,6 @@ public: void write (const char *buf, long length_buf) override; - void puts (const char *) override; - private: /* The wrapped raw output stream. */ ui_file *m_raw; diff --git a/gdb/serial.c b/gdb/serial.c index 831d2eb..b48b977 100644 --- a/gdb/serial.c +++ b/gdb/serial.c @@ -251,7 +251,7 @@ serial_open_ops_1 (const struct serial_ops *ops, const char *open_name) if (serial_logfile != NULL) { - std::unique_ptr file (new stdio_file ()); + stdio_file_up file (new stdio_file ()); if (!file->open (serial_logfile, "w")) perror_with_name (serial_logfile); diff --git a/gdb/ui-file.h b/gdb/ui-file.h index 3972d50..1cb9693 100644 --- a/gdb/ui-file.h +++ b/gdb/ui-file.h @@ -21,13 +21,13 @@ #include -/* The ui_file base class. */ +/* The abstract ui_file base class. */ class ui_file { public: ui_file (); - virtual ~ui_file (); + virtual ~ui_file () = 0; /* Public non-virtual API. */ @@ -117,8 +117,16 @@ public: /* string_file-specific public API. */ /* Accesses the std::string containing the entire output collected - so far. Returns a non-const reference so that it's easy to move - the string contents out of the string_file. */ + so far. + + Returns a non-const reference so that it's easy to move the + string contents out of the string_file. E.g.: + + string_file buf; + buf.printf (....); + buf.printf (....); + return std::move (buf.string ()); + */ std::string &string () { return m_string; } /* Provide a few convenience methods with the same API as the @@ -187,6 +195,8 @@ private: bool m_close_p; }; +typedef std::unique_ptr stdio_file_up; + /* Like stdio_file, but specifically for stderr. This exists because there is no real line-buffering on Windows, see diff --git a/gdb/utils.h b/gdb/utils.h index aada0a4..f138702 100644 --- a/gdb/utils.h +++ b/gdb/utils.h @@ -66,9 +66,6 @@ char **gdb_buildargv (const char *); extern struct cleanup *make_cleanup_freeargv (char **); -struct ui_file; -extern struct cleanup *make_cleanup_ui_file_delete (struct ui_file *); - struct ui_out; extern struct cleanup * make_cleanup_ui_out_redirect_pop (struct ui_out *uiout); -- 2.5.5