From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17285 invoked by alias); 22 Feb 2018 18:43:28 -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 16939 invoked by uid 89); 22 Feb 2018 18:43:28 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-7.6 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_LOW,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= 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; Thu, 22 Feb 2018 18:43:25 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5F9ACFA46A; Thu, 22 Feb 2018 18:43:24 +0000 (UTC) Received: from localhost (unused-10-15-17-196.yyz.redhat.com [10.15.17.196]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4738F10A85AF; Thu, 22 Feb 2018 18:43:24 +0000 (UTC) From: Sergio Durigan Junior To: Joel Brobecker 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> <20180221075605.dujv7xwx45soccnc@adacore.com> Date: Thu, 22 Feb 2018 18:43:00 -0000 In-Reply-To: <20180221075605.dujv7xwx45soccnc@adacore.com> (Joel Brobecker's message of "Wed, 21 Feb 2018 11:56:05 +0400") Message-ID: <878tbk6f1v.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2018-02/txt/msg00324.txt.bz2 On Wednesday, February 21 2018, Joel Brobecker wrote: > Hi Sergio, Hi Joel, > > On Fri, Feb 09, 2018 at 08:42:40PM -0500, 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". >> >> 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: >> >> $ gdbserver :1234 a.out >> >> 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. >> >> 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. >> >> Regression tested on the BuildBot, without regressions. >> >> gdb/ChangeLog: >> 2018-02-09 Sergio Durigan Junior >> >> * Makefile.in (SFILES): Add "common/pathstuff.c". >> (HFILES_NO_SRCDIR): Add "common/pathstuff.h". >> (COMMON_OBS): Add "pathstuff.o". >> * common/pathstuff.c: New file. >> * common/pathstuff.h: New file. >> * utils.c (gdb_realpath): Move to "common/pathstuff.c". >> (gdb_realpath_keepfile): Likewise. >> (gdb_abspath): Likewise. >> * utils.h: Include "common/pathstuff.h". >> (gdb_realpath): Move to "common/pathstuff.h". >> (gdb_realpath_keepfile): Likewise. >> (gdb_abspath): Likewise. >> >> gdb/gdbserver/ChangeLog: >> 2018-02-09 Sergio Durigan Junior > > Thanks for doing this work! No problem. >> * Makefile.in (SFILES): Add "$(srcdir)/common/pathstuff.c". >> (OBJS): Add "pathstuff.o". >> +++ b/gdb/common/pathstuff.c >> @@ -0,0 +1,144 @@ > [...] >> +gdb::unique_xmalloc_ptr >> +gdb_realpath (const char *filename) > > I realize you are just moving the function, but the function's missing > some documentation. I think it would be useful to add. > > What lead me to this is the fact that there is one particularly > important element, is that this function used the current working > directory to expand relative paths. In the case of GDB, I verified > that when the user does a "cd DIR", GDB updates both current_directory > and its actual current working directory (in other words, we always > maintain the property current_directory == getcwd (). > > In GDBserver, however, it doesn't seem to be the case. So I think > we need to be explicit about that, because calls to gdb_realpath > and gdb_abspath with the same filename might actually return > the path to two different files if the conditions are right! Exactly. GDB has different concepts for "inferior CWD" and "GDB CWD". gdbserver, after my patch which implemented the inferior CWD handling, just cares about the "inferior CWD". But now, with the current patch we're discussing, gdbserver will have a "current_directory" variable and will be more aware of its own CWD. > Ideally, I think we would want gdb_realpath and gdb_abspath to > return the same value. But, if we are interested, I suggest we > discuss that separately from this thread. This is potentially > disruptive (and potentially in a good way ;-)). Yes, that's a good point. I will include a comment about this. >> +/* Return PATH in absolute form, performing tilde-expansion if necessary. >> + PATH cannot be NULL or the empty string. >> + This does not resolve symlinks however, use gdb_realpath for that. */ >> + >> +extern gdb::unique_xmalloc_ptr gdb_abspath (const char *path); > > Similar to the above, I think we should be clear that the expansion > of relative path is done relative to the current_directory (or > the current working directory if NULL). Something like that. Consider it done. Thanks, -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/