From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 67646 invoked by alias); 24 Jan 2017 19:11:37 -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 67138 invoked by uid 89); 24 Jan 2017 19:11:36 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.2 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=no version=3.3.2 spammy=Hx-languages-length:5075, H*f:sk:558e1e5, H*u:1.2.3, H*UA:1.2.3 X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 24 Jan 2017 19:11:34 +0000 Received: by simark.ca (Postfix, from userid 33) id AE0001E166; Tue, 24 Jan 2017 14:11:32 -0500 (EST) To: Pedro Alves Subject: Re: [PATCH v2 5/5] Eliminate make_cleanup_ui_file_delete / make ui_file a class hierarchy X-PHP-Originating-Script: 33:rcube.php MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Tue, 24 Jan 2017 19:11:00 -0000 From: Simon Marchi Cc: gdb-patches@sourceware.org In-Reply-To: <558e1e55-23eb-716e-8117-c18d7bded7b8@redhat.com> 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> Message-ID: <91ab941736dd82a711868ea56651c20d@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.2.3 X-IsSubscribed: yes X-SW-Source: 2017-01/txt/msg00503.txt.bz2 On 2017-01-23 18:18, Pedro Alves wrote: > diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c > index ea64220..cc14d9d 100644 > --- a/gdb/aix-thread.c > +++ b/gdb/aix-thread.c > @@ -1762,43 +1762,42 @@ aix_thread_extra_thread_info (struct target_ops > *self, > if (!PD_TID (thread->ptid)) > return NULL; > > - buf = mem_fileopen (); > + string_file buf; You did not remove the old declaration of buf. > > pdtid = thread->priv->pdtid; > tid = thread->priv->tid; > > if (tid != PTHDB_INVALID_TID) > /* i18n: Like "thread-identifier %d, [state] running, suspended" > */ > - fprintf_unfiltered (buf, _("tid %d"), (int)tid); > + buf.printf (_("tid %d"), (int)tid); > > status = pthdb_pthread_state (pd_session, pdtid, &state); > if (status != PTHDB_SUCCESS) > state = PST_NOTSUP; > - fprintf_unfiltered (buf, ", %s", state2str (state)); > + buf.printf (", %s", state2str (state)); > > status = pthdb_pthread_suspendstate (pd_session, pdtid, > &suspendstate); > if (status == PTHDB_SUCCESS && suspendstate == PSS_SUSPENDED) > /* i18n: Like "Thread-Id %d, [state] running, suspended" */ > - fprintf_unfiltered (buf, _(", suspended")); > + buf.printf (_(", suspended")); > > status = pthdb_pthread_detachstate (pd_session, pdtid, > &detachstate); > if (status == PTHDB_SUCCESS && detachstate == PDS_DETACHED) > /* i18n: Like "Thread-Id %d, [state] running, detached" */ > - fprintf_unfiltered (buf, _(", detached")); > + buf.printf (_(", detached")); > > pthdb_pthread_cancelpend (pd_session, pdtid, &cancelpend); > if (status == PTHDB_SUCCESS && cancelpend) > /* i18n: Like "Thread-Id %d, [state] running, cancel pending" */ > - fprintf_unfiltered (buf, _(", cancel pending")); > + buf.printf (_(", cancel pending")); > > - ui_file_write (buf, "", 1); > + buf.write ("", 1); > > xfree (ret); /* Free old buffer. */ > > - ret = ui_file_xstrdup (buf, NULL); > - ui_file_delete (buf); > + ret = xstrdup (buf.string ()); Can xstrdup take an std::string? Implicit conversions only go the other way I think. It might be worth sending the series on the buildbot to test AIX. > > return ret; > } > diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c > index 2bdfa57..bc170b1 100644 > --- a/gdb/arm-tdep.c > +++ b/gdb/arm-tdep.c > @@ -9573,7 +9573,6 @@ extern initialize_file_ftype > _initialize_arm_tdep; /* -Wmissing-prototypes */ > void > _initialize_arm_tdep (void) > { > - struct ui_file *stb; > long length; > const char *setname; > const char *setdesc; > @@ -9645,13 +9644,12 @@ _initialize_arm_tdep (void) > valid_disassembly_styles[num_disassembly_options] = NULL; > > /* Create the help text. */ > - stb = mem_fileopen (); > - fprintf_unfiltered (stb, "%s%s%s", > - _("The valid values are:\n"), > - regdesc, > - _("The default is \"std\".")); > - helptext = ui_file_as_string (stb); > - ui_file_delete (stb); > + string_file stb; > + stb.printf ("%s%s%s", > + _("The valid values are:\n"), > + regdesc, > + _("The default is \"std\".")); > + helptext = std::move (stb.string ()); I think having that static helptext variable is useless here. The help text is copied in add_setshow_enum_cmd, so we don't need to keep helptext for the whole lifetime of the program. We could pass stb.c_str () directly to add_setshow_enum_cmd. > +class string_file : public ui_file > +{ > +public: > + string_file () {} > + ~string_file () override; > > -/* Returns a std::string containing the entire contents of FILE (as > - determined by ui_file_put()). */ > -extern std::string ui_file_as_string (struct ui_file *file); > + /* Override ui_file methods. */ > > -/* Similar to ui_file_xstrdup, but return a new string allocated on > - OBSTACK. */ > -extern char *ui_file_obsavestring (struct ui_file *file, > - struct obstack *obstack, long *length); > + void write (const char *buf, long length_buf) override; > > -extern long ui_file_read (struct ui_file *file, char *buf, long > length_buf); > + long read (char *buf, long length_buf) override > + { gdb_assert_not_reached ("a string_file is not readable"); } > + > + /* 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. */ I didn't understand what you meant by "to move the string contents out..." until I saw this helptext = std::move (stb.string ()); If it had been "to std::move the string contents out..." it might have been a bit clearer, I'm not entirely sure. > --- a/gdb/utils.c > +++ b/gdb/utils.c > @@ -187,18 +187,6 @@ make_cleanup_obstack_free (struct obstack > *obstack) > return make_cleanup (do_obstack_free, obstack); > } > > -static void > -do_ui_file_delete (void *arg) > -{ > - ui_file_delete ((struct ui_file *) arg); > -} > - > -struct cleanup * > -make_cleanup_ui_file_delete (struct ui_file *arg) > -{ > - return make_cleanup (do_ui_file_delete, arg); > -} > - You can remove the declaration in utils.h for this. That's all for me. Thanks! Simon