From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 92891 invoked by alias); 4 Aug 2017 14:52:08 -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 92868 invoked by uid 89); 4 Aug 2017 14:52:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.8 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=Hx-languages-length:3248 X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 04 Aug 2017 14:52:06 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id v74EpxvB017689 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 4 Aug 2017 10:52:04 -0400 Received: by simark.ca (Postfix, from userid 112) id E647C1EA05; Fri, 4 Aug 2017 10:51:59 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id 75FBE1E5A6; Fri, 4 Aug 2017 10:51:49 -0400 (EDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Fri, 04 Aug 2017 14:52:00 -0000 From: Simon Marchi To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [RFA] Use gdb::unique_xmalloc_ptr when calling tilde_expand In-Reply-To: <87bmnv8n2y.fsf@tromey.com> References: <20170803214348.27356-1-tom@tromey.com> <87bmnv8n2y.fsf@tromey.com> Message-ID: X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.0 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Fri, 4 Aug 2017 14:52:00 +0000 X-IsSubscribed: yes X-SW-Source: 2017-08/txt/msg00077.txt.bz2 On 2017-08-04 15:44, Tom Tromey wrote: >>>>>> "Simon" == Simon Marchi writes: > > Simon> The patch looks good to me. I noted two formatting nits (that > were > Simon> present before this patch), could you fix them before pushing? > > I did this. I don't really like fixing random things in the context of > a patch. Normally in fact I would expect patches containing unrelated > changes like that to be rejected on the basis of not being > single-purpose. I agree with you on the idea, we just have different criteria about what unrelated means. In this case since you're touching the source line inside, I would say that it's ok to fix the surrounding formatting at the same time. Anyhow, not a big deal. > Simon> Hmm, the old code discarded the cleanup and did an xstrdup, that > seems > Simon> like a leak that you fixed :) > > Yeah. I found another leak as well, but I haven't sent the patch for > that yet. > > I noticed I sent the wrong patch -- it didn't include all the changes > to > solib.c. So, I'm re-sending it rather than checking it in. The > additional changes were part of the buildbot try run though; they were > just an earlier commit I forgot to squash. Ok, I assume everything except solib.c is unchanged, so I'll just take a look at solib.c. > diff --git a/gdb/solib.c b/gdb/solib.c > index 5b538eb..195183d 100644 > --- a/gdb/solib.c > +++ b/gdb/solib.c > @@ -546,14 +546,10 @@ static int > solib_map_sections (struct so_list *so) > { > const struct target_so_ops *ops = solib_ops (target_gdbarch ()); > - char *filename; > struct target_section *p; > - struct cleanup *old_chain; > > - filename = tilde_expand (so->so_name); > - old_chain = make_cleanup (xfree, filename); > - gdb_bfd_ref_ptr abfd (ops->bfd_open (filename)); > - do_cleanups (old_chain); > + gdb::unique_xmalloc_ptr filename (tilde_expand (so->so_name)); > + gdb_bfd_ref_ptr abfd (ops->bfd_open (filename.get ())); > > if (abfd == NULL) > return 0; > @@ -1301,23 +1297,22 @@ static void > reload_shared_libraries_1 (int from_tty) > { > struct so_list *so; > - struct cleanup *old_chain = make_cleanup (null_cleanup, NULL); > > if (print_symbol_loading_p (from_tty, 0, 0)) > printf_unfiltered (_("Loading symbols for shared libraries.\n")); > > for (so = so_list_head; so != NULL; so = so->next) > { > - char *filename, *found_pathname = NULL; > + char *found_pathname = NULL; > int was_loaded = so->symbols_loaded; > symfile_add_flags add_flags = SYMFILE_DEFER_BP_RESET; > > if (from_tty) > add_flags |= SYMFILE_VERBOSE; > > - filename = tilde_expand (so->so_original_name); > - make_cleanup (xfree, filename); > - gdb_bfd_ref_ptr abfd (solib_bfd_open (filename)); > + gdb::unique_xmalloc_ptr filename > + (tilde_expand (so->so_original_name)); > + gdb_bfd_ref_ptr abfd (solib_bfd_open (filename.get ())); > if (abfd != NULL) > { > found_pathname = xstrdup (bfd_get_filename (abfd.get ())); > @@ -1364,8 +1359,6 @@ reload_shared_libraries_1 (int from_tty) > solib_read_symbols (so, add_flags); > } > } > - > - do_cleanups (old_chain); There is still a cleanup in reload_shared_libraries_1, so I don't think the do_cleanups should be removed. Simon