From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 109409 invoked by alias); 12 Feb 2018 19:01:41 -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 109400 invoked by uid 89); 12 Feb 2018 19:01:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-27.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_LOW,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=sunday, Sunday X-HELO: mx1.redhat.com Received: from mx3-rdu2.redhat.com (HELO mx1.redhat.com) (66.187.233.73) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 12 Feb 2018 19:01:39 +0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7AA44404084F; Mon, 12 Feb 2018 19:01:37 +0000 (UTC) Received: from localhost (unused-10-15-17-193.yyz.redhat.com [10.15.17.193]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 503DE2166BAE; Mon, 12 Feb 2018 19:01:37 +0000 (UTC) From: Sergio Durigan Junior To: Simon Marchi Cc: GDB Patches , Simon Marchi Subject: Re: [PATCH 1/2] Create new common/pathstuff.[ch] References: <20180210014241.19278-1-sergiodj@redhat.com> <20180210014241.19278-2-sergiodj@redhat.com> <084a12bf-6b54-9399-e4fe-887952a57bc1@simark.ca> Date: Mon, 12 Feb 2018 19:01:00 -0000 In-Reply-To: <084a12bf-6b54-9399-e4fe-887952a57bc1@simark.ca> (Simon Marchi's message of "Sun, 11 Feb 2018 17:13:58 -0500") Message-ID: <877eri9gms.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2018-02/txt/msg00175.txt.bz2 On Sunday, February 11 2018, Simon Marchi wrote: > On 2018-02-09 08:42 PM, Sergio Durigan Junior wrote: >> This commit moves the path manipulation routines found on utils.c to a >> new common/pathstuff.c, and updates the Makefile.in's accordingly. >> The routines moved are "gdb_realpath", "gdb_realpath_keepfile" and >> "gdb_abspath". >>=20 >> This will be needed because gdbserver will have to call "gdb_abspath" >> on my next patch, which implements a way to expand the path of the >> inferior provided by the user in order to allow specifying just the >> binary name when starting gdbserver, like: >>=20 >> $ gdbserver :1234 a.out >>=20 >> With the recent addition of the startup-with-shell feature on >> gdbserver, this scenario doesn't work anymore if the user doesn't have >> the current directory listed in the PATH variable. >>=20 >> I had to do a minor adjustment on "gdb_abspath" because we don't have >> access to "tilde_expand" on gdbserver, so now the function is using >> "gdb_tilde_expand" instead. Otherwise, the code is the same. >>=20 >> Regression tested on the BuildBot, without regressions. > > Hi Sergio, Hey, Simon, > Thanks for looking into this! My pleasure. > This commit does not build: > > /home/simark/src/binutils-gdb/gdb/common/pathstuff.c: In function =E2=80= =98gdb::unique_xmalloc_ptr gdb_abspath(const char*)=E2=80=99: > /home/simark/src/binutils-gdb/gdb/common/pathstuff.c:140:14: error: =E2= =80=98current_directory=E2=80=99 was not declared in this scope > (concat (current_directory, > ^~~~~~~~~~~~~~~~~ > /home/simark/src/binutils-gdb/gdb/common/pathstuff.c:140:14: note: sugges= ted alternative: =E2=80=98read_direction=E2=80=99 > (concat (current_directory, > ^~~~~~~~~~~~~~~~~ > read_direction Ah. Sorry about that; that's the problem of testing on BuildBot with everything-in-one-patch. I'll include the declaration of current_directory in common-defs.h and also in gdbserver/server.c. > I guess you need to move the declaration to common-defs.h in this commit = instead of > the next one. I also got this whitespace error from git am: > > .git/rebase-apply/patch:131: trailing whitespace. > /* Extract the basename of filename, and return immediately > > I think it's in code you copied, but if you can remove the extra space wh= ile at it > it would be nice. Sure. >> +/* See common/pathstuff.h. */ >> + >> +gdb::unique_xmalloc_ptr >> +gdb_abspath (const char *path) >> +{ >> + gdb_assert (path !=3D NULL && path[0] !=3D '\0'); >> + >> + if (path[0] =3D=3D '~') >> + { >> + std::string new_path =3D gdb_tilde_expand (path); >> + >> + return gdb::unique_xmalloc_ptr (xstrdup (new_path.c_str ())= ); > > We should try to avoid unnecessary copies, when possible. Here, we could= either make > another version of gdb_tilde_expand (it would have to be another name) th= at returns > a gdb::unique_xmalloc_ptr or make gdb_abspath return an std::string= . I think the > former would be better for now because some callers require a gdb::unique= _xmalloc_ptr, > and would have to do a copy themselves. So using a gdb::unique_xmalloc_p= tr across the > whole chain would give the least amount of copies. OK. >> diff --git a/gdb/utils.h b/gdb/utils.h >> index b234762929..4f25be0968 100644 >> --- a/gdb/utils.h >> +++ b/gdb/utils.h >> @@ -23,6 +23,7 @@ >>=20=20 >> #include "exceptions.h" >> #include "common/scoped_restore.h" >> +#include "common/pathstuff.h" > > I don't think utils.h should be including common/pathstuff.h, because it = is not using it. > I understand why you did this (this ensures that every current user of th= ese functions > will automatically see the new declaration), but in the long term I think= it's better if > the users include "pathstuff.h". The files that use these functions are: > > $ grep -e gdb_realpath_keepfile -e gdb_realpath -e gdb_abspath *.c */*.c = -l | sort > auto-load.c > common/pathstuff.c > compile/compile.c > dwarf2read.c > exec.c > guile/scm-safe-call.c > linux-thread-db.c > main.c > nto-tdep.c > objfiles.c > source.c > symtab.c > utils.c > > They are all files included in a --enable-targets=3Dall build, so it shou= ld be easy to > find where #include "common/pathstuff.h" is missing. Alright, no problem. I'll send a v2 soon. Thanks, --=20 Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/