From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 130395 invoked by alias); 28 Jan 2019 20:41:00 -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 130387 invoked by uid 89); 28 Jan 2019 20:40:59 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx2.freebsd.org Received: from mx2.freebsd.org (HELO mx2.freebsd.org) (8.8.178.116) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 28 Jan 2019 20:40:58 +0000 Received: from mx1.freebsd.org (mx1.freebsd.org [96.47.72.80]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (Client CN "mx1.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx2.freebsd.org (Postfix) with ESMTPS id 92B1294843; Mon, 28 Jan 2019 20:40:56 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id C83F176FB1; Mon, 28 Jan 2019 20:40:55 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from John-Baldwins-MacBook-Pro-3.local (ralph.baldwin.cx [66.234.199.215]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) (Authenticated sender: jhb) by smtp.freebsd.org (Postfix) with ESMTPSA id 6E1298574; Mon, 28 Jan 2019 20:40:55 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Subject: Re: [PATCH 2/2] Trim trailing directory separators from new sysroots. To: Simon Marchi , gdb-patches@sourceware.org References: <2c219947da76ada8ee0596dc61a4abef3ebdc256.1548205042.git.jhb@FreeBSD.org> <1577437a-a984-4401-216f-dca1e0b18872@simark.ca> From: John Baldwin Openpgp: preference=signencrypt Message-ID: <79e03f4a-6b48-4d30-8427-a7594bc38bb6@FreeBSD.org> Date: Mon, 28 Jan 2019 20:41:00 -0000 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: <1577437a-a984-4401-216f-dca1e0b18872@simark.ca> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: C83F176FB1 X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-2.92 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_HAM_MEDIUM(-0.99)[-0.994,0]; NEURAL_HAM_SHORT(-0.92)[-0.925,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US]; NEURAL_HAM_LONG(-1.00)[-1.000,0] X-IsSubscribed: yes X-SW-Source: 2019-01/txt/msg00588.txt.bz2 On 1/26/19 11:10 AM, Simon Marchi wrote: > On 2019-01-22 8:03 p.m., John Baldwin wrote: >> The separate debugfile logic assumes that the sysroot does not include >> a trailing separator (all of the sysroot logic in >> find_separate_debug_file requires the next character after the sysroot >> in a object file path to be a directory separator). However, normal >> filename completion will result in setting a sysroot with a trailing >> directory separator. If the directory portion of a new sysroot ends >> with a directory separator and is not specifying the root directory, >> trim the trailing separator. This permits the sysroot logic to work >> the same if a sysroot of "/myroot/" is specified instead of "/myroot". >> >> Note that the sysroot displayed by 'show sysroot' will reflect the >> trimmed sysroot. > > I am not sure about this, it seems fragile to do this one off thing. The > point where the path is stripped is quite far from the point where it has > an impact, so it's really not obvious why we do it (though this could be > fixed by saying why we do it in the comment). > > I think it would be a step in a better direction to have an "is_child_path" > function that returns whether a path is a child of another one. It would > be responsible for handling the case of the parent potentially having a > trailing directory separator. The implementation of such function might > get hairy, but the advantage is that it should be pretty easy to unit > test. > > Finally, I think it would make this code more obvious: > > if (canon_dir != NULL > && filename_ncmp (canon_dir, gdb_sysroot, > strlen (gdb_sysroot)) == 0 > && IS_DIR_SEPARATOR (canon_dir[strlen (gdb_sysroot)])) > > vs > > if (canon_dir != NULL && is_child_patn (gdb_sysroot, canon_dir)) I have taken this approach, but it ended up being a bit more than just a boolean check as I needed to reliably get the base path of the child component as the existing 'canon_dir + strlen (gdb_sysroot)' assumed no trailing separator in gdb_sysroot. I'll post the updated version along with another fix I encountered in some more testing today as a v2 in a sec. -- John Baldwin                                                                            Â