From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8628 invoked by alias); 25 Jan 2017 18:28:45 -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 8605 invoked by uid 89); 25 Jan 2017 18:28:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-5.1 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=H*MI:sk:91ab941, H*f:sk:91ab941, H*i:sk:91ab941, you! 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 18:28:43 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (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 7C14051452; Wed, 25 Jan 2017 18:28:43 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.phx2.redhat.com [10.5.9.4]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v0PISdDD027573; Wed, 25 Jan 2017 13:28:40 -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> <91ab941736dd82a711868ea56651c20d@polymtl.ca> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: <0a004b52-a388-e7ec-fe3e-ccac3e31f7b2@redhat.com> Date: Wed, 25 Jan 2017 18:28: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: <91ab941736dd82a711868ea56651c20d@polymtl.ca> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-01/txt/msg00549.txt.bz2 On 01/24/2017 07:11 PM, Simon Marchi wrote: > Can xstrdup take an std::string? Implicit conversions only go the other > way I think. Meh. No, that's broken. I'll fix it. > > It might be worth sending the series on the buildbot to test AIX. I'll try building this on gcc119 (AIX 7.2) on the compile farm. >From the discussion on gdb@, the bot is broken. :-/ >> + 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. OK. While at it, this can actually be a string_printf instead of a string_file. I've made that change. > >> +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. That would read a bit odd to me, since "std::move" itself does nothing, it's just a cast. Maybe it'll be clearer with an example? Something like: /* 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; } > >> --- 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. Indeed. Fixed. > > That's all for me. > > Thanks! Thank you! -- Pedro Alves