From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 92496 invoked by alias); 26 Jan 2019 19:10:29 -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 92484 invoked by uid 89); 26 Jan 2019 19:10:28 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 26 Jan 2019 19:10:27 +0000 Received: from [10.0.0.178] (unknown [192.222.164.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 811731E4A3; Sat, 26 Jan 2019 14:10:25 -0500 (EST) Subject: Re: [PATCH 2/2] Trim trailing directory separators from new sysroots. To: John Baldwin , gdb-patches@sourceware.org References: <2c219947da76ada8ee0596dc61a4abef3ebdc256.1548205042.git.jhb@FreeBSD.org> From: Simon Marchi Message-ID: <1577437a-a984-4401-216f-dca1e0b18872@simark.ca> Date: Sat, 26 Jan 2019 19:10:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: <2c219947da76ada8ee0596dc61a4abef3ebdc256.1548205042.git.jhb@FreeBSD.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2019-01/txt/msg00566.txt.bz2 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)) Simon