From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x433.google.com (mail-wr1-x433.google.com [IPv6:2a00:1450:4864:20::433]) by sourceware.org (Postfix) with ESMTPS id 8754E3858002 for ; Tue, 7 Sep 2021 14:12:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8754E3858002 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=embecosm.com Received: by mail-wr1-x433.google.com with SMTP id z4so14670589wrr.6 for ; Tue, 07 Sep 2021 07:12:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=FqhkdIQ66G3H77grPUqgFmQsGR+3UQFPbvu6kWH1AqQ=; b=WbV9XX+H3K+P5KszrDgsJMy48+DgmX1z5ykEPV/ftYlGPK/ZohLAIiRk09c0zngfGv WoQwudE/QlvZ+8N8Sv+TlKMAZo3s4nZGu1pTRDkuUTOjzwmpAGpIn8IqcKJh1bfrE5Bc aSFKYTE/eFOUa9+D5ja17XfdBkGcaFm2zpm56OJNp0dWAV6p5py63gUybj4vmGTfieu7 a91eXu2wRB84L6WP854UVind1fC7M1TZD3fp5QcUNNHuxx+yEzSVC7i6/5YKvXvMul11 RAZoYpFjs4UQm7KqLTNTlyfxr/AG95ulhpXjZ7xbeEeUCr3/clio5ps5mMH/MIUlTSQg zrxw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=FqhkdIQ66G3H77grPUqgFmQsGR+3UQFPbvu6kWH1AqQ=; b=bxJLGYprmZYrPhSCkl7EuSPdzLSqL+L7zuNay3+Mkm3eLv+LcWoLlGbZQdcqQQRquU mS6PSSiP77C/0iXtUYC3OLkLX5PSp8Cc0S9fia3o6nxCA1WSk3BY65VDxwYb/SJtfyfV cfW9TRNOUH5Ypc7XiVEqTboxPoRRKUJGte7JWLhZfgEHeq+cfO4n9M7rkaZPHkheX5Ux IKDNhxLY7dEASV1F7u1SDbIVLcOO96oGgQ1xxprOHx/3Yrpk+o0/Lq1DbPqs80Dz1q01 He+uHreJ/TkCCwlXA7g1UfzO42FwkNsIDupbeOM5RUv0ny1ii6VljjQzBvXw/WtqRAAI AACg== X-Gm-Message-State: AOAM531mUYkOvnEBaOtrloUUO3Oj/yfPn3goABfrdNRvSoli4W55VnX9 Uirviu0HK/Nkld0pRPIvLLA7ypZVThtbuQ== X-Google-Smtp-Source: ABdhPJwYrzNl5Xe0fUf65rZsgGvwja06veatEqExjg0WQ5nj9+BtPrG9W3j+hH7tSEVrxzoIHRG42Q== X-Received: by 2002:a05:6000:1569:: with SMTP id 9mr19433736wrz.242.1631023942657; Tue, 07 Sep 2021 07:12:22 -0700 (PDT) Received: from localhost (host109-154-20-132.range109-154.btcentralplus.com. [109.154.20.132]) by smtp.gmail.com with ESMTPSA id z19sm3041541wma.0.2021.09.07.07.12.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 07 Sep 2021 07:12:22 -0700 (PDT) Date: Tue, 7 Sep 2021 15:12:21 +0100 From: Andrew Burgess To: Simon Marchi Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 2/6] gdb: make use of std::string in utils.c Message-ID: <20210907141221.GS2581@embecosm.com> References: <0f6f8619349c7e60e478ee2e47d3bc76ebaff416.1629366146.git.andrew.burgess@embecosm.com> <692bd212-465d-aa25-338f-ce239d0c57e2@polymtl.ca> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <692bd212-465d-aa25-338f-ce239d0c57e2@polymtl.ca> X-Operating-System: Linux/5.8.18-100.fc31.x86_64 (x86_64) X-Uptime: 15:11:36 up 21 days, 3:07, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 07 Sep 2021 14:12:25 -0000 * Simon Marchi [2021-08-19 14:41:41 -0400]: > > > On 2021-08-19 5:49 a.m., Andrew Burgess wrote: > > Replace use of manual string management (malloc/free) with std::string > > when creating commands in utils.c. Things are a little bit messy as > > creating the prefix commands (using add_basic_prefix_cmd and > > add_show_prefix_cmd), don't copy the doc string, while creating the > > actual set/show commands does copy the doc string, this explains the > > extra xstrdup when creating the prefix commands. > > Indeed, all of this is really not consistent. I guess we would need to > create versions of the add_*_cmd that take a > gdb::unique_xmalloc_pointer and set > cmd_list_element::doc_allocated. > > > There should be no user visible changes after this commit. > > --- > > gdb/utils.c | 58 ++++++++++++++++++++++++----------------------------- > > 1 file changed, 26 insertions(+), 32 deletions(-) > > > > diff --git a/gdb/utils.c b/gdb/utils.c > > index 3916ae5a1c9..143c2eddd70 100644 > > --- a/gdb/utils.c > > +++ b/gdb/utils.c > > @@ -508,72 +508,66 @@ add_internal_problem_command (struct internal_problem *problem) > > { > > struct cmd_list_element **set_cmd_list; > > struct cmd_list_element **show_cmd_list; > > - char *set_doc; > > - char *show_doc; > > > > set_cmd_list = XNEW (struct cmd_list_element *); > > show_cmd_list = XNEW (struct cmd_list_element *); > > *set_cmd_list = NULL; > > *show_cmd_list = NULL; > > > > - set_doc = xstrprintf (_("Configure what GDB does when %s is detected."), > > - problem->name); > > + std::string set_doc > > + = string_printf (_("Configure what GDB does when %s is detected."), > > + problem->name); > > > > - show_doc = xstrprintf (_("Show what GDB does when %s is detected."), > > - problem->name); > > + std::string show_doc > > + = string_printf (_("Show what GDB does when %s is detected."), > > + problem->name); > > > > - add_basic_prefix_cmd (problem->name, class_maintenance, set_doc, > > - set_cmd_list, > > + add_basic_prefix_cmd (problem->name, class_maintenance, > > + xstrdup (set_doc.c_str ()), set_cmd_list, > > 0/*allow-unknown*/, &maintenance_set_cmdlist); > > > > - add_show_prefix_cmd (problem->name, class_maintenance, show_doc, > > - show_cmd_list, > > + add_show_prefix_cmd (problem->name, class_maintenance, > > + xstrdup (show_doc.c_str ()), show_cmd_list, > > 0/*allow-unknown*/, &maintenance_show_cmdlist); > > I guess you could keep using xstrprintf for these to avoid the extra > copy... but what you have LGTM either way. I've taken your advice. Below is what I pushed. Thanks, Andrew -- commit 747656685b3e8477868478cd927fbb2834937aff Author: Andrew Burgess Date: Tue Aug 17 13:29:22 2021 +0100 gdb: make use of std::string in utils.c Replace some of the manual string management (malloc/free) with std::string when creating commands in utils.c. Things are a little bit messy as, creating the prefix commands (using add_basic_prefix_cmd and add_show_prefix_cmd), doesn't copy the doc string, while creating the actual set/show commands (using add_setshow_enum_cmd) does copy the doc string. As a result, I have retained the use of xstrprintf when creating the prefix command doc strings, but switched to using std::string when creating the actual set/show commands. There should be no user visible changes after this commit. diff --git a/gdb/utils.c b/gdb/utils.c index 0009cb10d87..0a7c270b40d 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -508,19 +508,21 @@ add_internal_problem_command (struct internal_problem *problem) { struct cmd_list_element **set_cmd_list; struct cmd_list_element **show_cmd_list; - char *set_doc; - char *show_doc; set_cmd_list = XNEW (struct cmd_list_element *); show_cmd_list = XNEW (struct cmd_list_element *); *set_cmd_list = NULL; *show_cmd_list = NULL; - set_doc = xstrprintf (_("Configure what GDB does when %s is detected."), - problem->name); - - show_doc = xstrprintf (_("Show what GDB does when %s is detected."), - problem->name); + /* The add_basic_prefix_cmd and add_show_prefix_cmd functions take + ownership of the string passed in, which is why we don't need to free + set_doc and show_doc in this function. */ + const char *set_doc + = xstrprintf (_("Configure what GDB does when %s is detected."), + problem->name); + const char *show_doc + = xstrprintf (_("Show what GDB does when %s is detected."), + problem->name); add_basic_prefix_cmd (problem->name, class_maintenance, set_doc, set_cmd_list, @@ -532,48 +534,42 @@ add_internal_problem_command (struct internal_problem *problem) if (problem->user_settable_should_quit) { - set_doc = xstrprintf (_("Set whether GDB should quit " - "when an %s is detected."), - problem->name); - show_doc = xstrprintf (_("Show whether GDB will quit " - "when an %s is detected."), - problem->name); + std::string set_quit_doc + = string_printf (_("Set whether GDB should quit when an %s is " + "detected."), problem->name); + std::string show_quit_doc + = string_printf (_("Show whether GDB will quit when an %s is " + "detected."), problem->name); add_setshow_enum_cmd ("quit", class_maintenance, internal_problem_modes, &problem->should_quit, - set_doc, - show_doc, + set_quit_doc.c_str (), + show_quit_doc.c_str (), NULL, /* help_doc */ NULL, /* setfunc */ NULL, /* showfunc */ set_cmd_list, show_cmd_list); - - xfree (set_doc); - xfree (show_doc); } if (problem->user_settable_should_dump_core) { - set_doc = xstrprintf (_("Set whether GDB should create a core " - "file of GDB when %s is detected."), - problem->name); - show_doc = xstrprintf (_("Show whether GDB will create a core " - "file of GDB when %s is detected."), - problem->name); + std::string set_core_doc + = string_printf (_("Set whether GDB should create a core file of " + "GDB when %s is detected."), problem->name); + std::string show_core_doc + = string_printf (_("Show whether GDB will create a core file of " + "GDB when %s is detected."), problem->name); add_setshow_enum_cmd ("corefile", class_maintenance, internal_problem_modes, &problem->should_dump_core, - set_doc, - show_doc, + set_core_doc.c_str (), + show_core_doc.c_str (), NULL, /* help_doc */ NULL, /* setfunc */ NULL, /* showfunc */ set_cmd_list, show_cmd_list); - - xfree (set_doc); - xfree (show_doc); } }