From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 101582 invoked by alias); 13 Mar 2018 02:37:47 -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 101438 invoked by uid 89); 13 Mar 2018 02:37:38 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_STOCKGEN,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= 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, 13 Mar 2018 02:37:34 +0000 Received: from [10.0.0.11] (192-222-251-162.qc.cable.ebox.net [192.222.251.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id CD7BD1E481; Mon, 12 Mar 2018 22:37:32 -0400 (EDT) Subject: Re: [RFA] Remove two cleanups using std::string To: Tom Tromey , gdb-patches@sourceware.org References: <20180312223829.7108-1-tom@tromey.com> From: Simon Marchi Message-ID: Date: Tue, 13 Mar 2018 02:37:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <20180312223829.7108-1-tom@tromey.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-03/txt/msg00263.txt.bz2 On 2018-03-12 06:38 PM, Tom Tromey wrote: > This patches removes cleanups from a couple of spots by using > std::string rather than manual memory management. > > Regression tested by the buildbot, though note that I don't believe > the buildbot actually exercises the machoread code. > > gdb/ChangeLog > 2018-03-12 Tom Tromey > > * machoread.c (macho_check_dsym): Change filenamep to a > std::string*. > (macho_symfile_read): Update. > * symfile.c (load_command): Use std::string. > --- > gdb/ChangeLog | 7 +++++++ > gdb/machoread.c | 12 +++++------- > gdb/symfile.c | 44 ++++++++++++++------------------------------ > 3 files changed, 26 insertions(+), 37 deletions(-) > > diff --git a/gdb/machoread.c b/gdb/machoread.c > index b270675d61..ab4410c925 100644 > --- a/gdb/machoread.c > +++ b/gdb/machoread.c > @@ -750,12 +750,12 @@ macho_symfile_read_all_oso (VEC (oso_el) **oso_vector_ptr, > #define DSYM_SUFFIX ".dSYM/Contents/Resources/DWARF/" > > /* Check if a dsym file exists for OBJFILE. If so, returns a bfd for it > - and return *FILENAMEP with its original xmalloc-ated filename. > + and return *FILENAMEP with its original filename. > Return NULL if no valid dsym file is found (FILENAMEP is not used in > such case). */ > > static gdb_bfd_ref_ptr > -macho_check_dsym (struct objfile *objfile, char **filenamep) > +macho_check_dsym (struct objfile *objfile, std::string *filenamep) > { > size_t name_len = strlen (objfile_name (objfile)); > size_t dsym_len = strlen (DSYM_SUFFIX); > @@ -804,7 +804,7 @@ macho_check_dsym (struct objfile *objfile, char **filenamep) > objfile_name (objfile)); > return NULL; > } > - *filenamep = xstrdup (dsym_filename); > + *filenamep = std::string (dsym_filename); > return dsym_bfd; > } > > @@ -821,7 +821,7 @@ macho_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags) > be in the executable. */ > if (bfd_get_file_flags (abfd) & (EXEC_P | DYNAMIC)) > { > - char *dsym_filename; > + std::string dsym_filename; > > /* Process the normal symbol table first. */ > storage_needed = bfd_get_symtab_upper_bound (objfile->obfd); > @@ -865,8 +865,6 @@ macho_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags) > { > struct bfd_section *asect, *dsect; > > - make_cleanup (xfree, dsym_filename); > - > if (mach_o_debug_level > 0) > printf_unfiltered (_("dsym file found\n")); > > @@ -882,7 +880,7 @@ macho_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags) > } > > /* Add the dsym file as a separate file. */ > - symbol_file_add_separate (dsym_bfd.get (), dsym_filename, > + symbol_file_add_separate (dsym_bfd.get (), dsym_filename.c_str (), > symfile_flags, objfile); > > /* Don't try to read dwarf2 from main file or shared libraries. */ > diff --git a/gdb/symfile.c b/gdb/symfile.c > index 58747d545b..f714845a6e 100644 > --- a/gdb/symfile.c > +++ b/gdb/symfile.c > @@ -1789,8 +1789,6 @@ find_sym_fns (bfd *abfd) > static void > load_command (const char *arg, int from_tty) > { > - struct cleanup *cleanup = make_cleanup (null_cleanup, NULL); > - > dont_repeat (); > > /* The user might be reloading because the binary has changed. Take > @@ -1798,40 +1796,28 @@ load_command (const char *arg, int from_tty) > reopen_exec_file (); > reread_symbols (); > > + std::string temp; > if (arg == NULL) > { > - const char *parg; > - int count = 0; > + const char *parg, *prev; > > - parg = arg = get_exec_file (1); > + arg = get_exec_file (1); > > - /* Count how many \ " ' tab space there are in the name. */ > + /* We may need to quote this string so buildargv can pull it > + apart. */ > + prev = parg = arg; > while ((parg = strpbrk (parg, "\\\"'\t "))) > { > - parg++; > - count++; > + temp.append (prev, parg - prev); > + prev = parg++; > + temp.push_back ('\\'); > } > - > - if (count) > + /* If we have not copied anything yet, then we didn't see a > + character to quote, and we can just leave ARG unchanged. */ > + if (!temp.empty ()) > { > - /* We need to quote this string so buildargv can pull it apart. */ > - char *temp = (char *) xmalloc (strlen (arg) + count + 1 ); > - char *ptemp = temp; > - const char *prev; > - > - make_cleanup (xfree, temp); > - > - prev = parg = arg; > - while ((parg = strpbrk (parg, "\\\"'\t "))) > - { > - strncpy (ptemp, prev, parg - prev); > - ptemp += parg - prev; > - prev = parg++; > - *ptemp++ = '\\'; > - } > - strcpy (ptemp, prev); > - > - arg = temp; > + temp.append (prev); > + arg = temp.c_str (); > } > } > > @@ -1840,8 +1826,6 @@ load_command (const char *arg, int from_tty) > /* After re-loading the executable, we don't really know which > overlays are mapped any more. */ > overlay_cache_invalid = 1; > - > - do_cleanups (cleanup); > } > > /* This version of "load" should be usable for any target. Currently > LGTM. I would be more confident if that non-trivial escaping code was unit-tested, but I tried it by hand and it seems to do what it's supposed to do. Simon