From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 45675 invoked by alias); 10 Sep 2019 19:56:19 -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 45666 invoked by uid 89); 10 Sep 2019 19:56:19 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-18.3 required=5.0 tests=AWL,BAYES_00,ENV_AND_HDR_SPF_MATCH,RCVD_IN_DNSWL_NONE,SPF_PASS,USER_IN_DEF_SPF_WL autolearn=ham version=3.3.1 spammy=management X-HELO: mail-oi1-f196.google.com Received: from mail-oi1-f196.google.com (HELO mail-oi1-f196.google.com) (209.85.167.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 10 Sep 2019 19:56:16 +0000 Received: by mail-oi1-f196.google.com with SMTP id z6so6678521oix.9 for ; Tue, 10 Sep 2019 12:56:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=fd4nmcjH58mCQ+TWOdWZnqbJ/7+WNRP+7Q14lMMtUxo=; b=iafWhX6SawPQdwvwn7rHECkkis4dIY1Ta9hfFa1Be9vBBXCV6uRzCDFj3Pcd+GIK8U XACzQPWdzkRJ2MV5+a2mmEenPa5gw0JfLW4hepFsZo/tc0Y1wpPss/e0tebNrdGxGHvD jmsbnwuCDUL5+Pe59t2U7TFbWRgMg/J7oqeegl7/2BBGDaQzjXyZRqbUxol5PjQAKeFu OTTCZH3OShSQtNqxM7OuTs2EQtqVd3xqOC9aWwh/88gkxn8pHJpBYVcWD2/XOQ31r9d+ RqIcvMTBunQ3Ba80yFNeJDuKruWC2KHPEbpMuHOztGOKJtw8ccSUNNVwrROK78Fu+bxy s3Fw== MIME-Version: 1.0 References: <20190909180830.215313-1-cbiesinger@google.com> <20190909180830.215313-4-cbiesinger@google.com> <87ftl46uke.fsf@tromey.com> In-Reply-To: <87ftl46uke.fsf@tromey.com> From: "Christian Biesinger via gdb-patches" Reply-To: Christian Biesinger Date: Tue, 10 Sep 2019 19:56:00 -0000 Message-ID: Subject: Re: [PATCH 3/3] Make relocate_{path,gdb_directory} return std::string To: Tom Tromey Cc: Christian Biesinger via gdb-patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2019-09/txt/msg00191.txt.bz2 On Tue, Sep 10, 2019 at 10:27 AM Tom Tromey wrote: > > >>>>> "Christian" == Christian Biesinger via gdb-patches writes: > > Christian> This simplifies memory management. I've also changed some global variables > Christian> to std::string accordingly (which store the result of these functions), > Christian> but not all because some are used with add_setshow_optional_filename_cmd > Christian> which requires a char*. > > Thank you. Thanks for the review! > Christian> - xfree (gdb_datadir); > Christian> - gdb_datadir = gdb_realpath (new_datadir).release (); > Christian> + gdb_datadir = gdb_realpath (new_datadir).get (); > > I wonder if using a unique_xmalloc_ptr would be better. > I suppose it doesn't matter hugely, but it would eliminate some of these > double allocations. Do you mean for the return type of relocate_path and relocate_gdb_directory? Hm.. I can change it if you want, but I'd rather not -- not all codepaths go through an xmalloc (when relocatable=false, or relocate_path returns null in relocate_gdb_directory). So that would add some xstrdup calls that would not otherwise be necessary. > Christian> + gdb::unique_xmalloc_ptr abs_datadir = > Christian> + gdb_abspath (gdb_datadir.c_str ()); > > "=" on continuation line. Done. > Christian> -static char * > Christian> +static std::string > Christian> relocate_path (const char *progname, const char *initial, bool relocatable) > Christian> { > Christian> if (relocatable) > Christian> - return make_relative_prefix (progname, BINDIR, initial); > Christian> - return xstrdup (initial); > Christian> + { > Christian> + char *str = make_relative_prefix (progname, BINDIR, initial); > Christian> + if (str != nullptr) > Christian> + return str; > > This seems to leak "str". Oh, thanks! Fixed using a unique_xmalloc_ptr. > Christian> + char *canon_sysroot = lrealpath (dir.c_str ()); > > Changing this to a unique_xmalloc_ptr would mean one less call to > xfree... Thanks, done. > Christian> + debug_file_directory = > > "=" on the next line, there is one more of these in this spot. Done. I'll send a new version with those comments fixed in a moment. Christian