From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f50.google.com (mail-wm1-f50.google.com [209.85.128.50]) by sourceware.org (Postfix) with ESMTPS id 2C8A03858C53 for ; Wed, 20 Apr 2022 11:52:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 2C8A03858C53 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wm1-f50.google.com with SMTP id ay11-20020a05600c1e0b00b0038eb92fa965so3491872wmb.4 for ; Wed, 20 Apr 2022 04:52:07 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=1Rvuy5R9167mB91+lSKZjkF7UBim79BuTVQiHAYMjfc=; b=A+8H0+z4WueR0bAyed2Hvu0fgOH82vWZbxwpMRWH4uTLHsfk7BEyy3Ee16VV6KUlmI TgSZ/owbNbNxTNvD7pAk3A57nZcYgwGWJWM0N9LC09kh9wdEpGSfDLyYgsX/cVwt7YVi XCLy7tDxcRvGK8unPf+q8Uw/QnqIYWhiPFwanCCYMn7LVNHdOwyd8XtZR7BC85byNc1n Mf3CsY55bua5w9of6WAMx7P4MjL5w2M61UIKX1t+c9IS4qZzPUsJpDmghYpv7Y08mvVw wjw8Ktusrgze9AMSuHeYlqihmvyGBDhy+LeS6XUKuK/INj8rfBmm5bftiX1TdZR6CDgo TciA== X-Gm-Message-State: AOAM530gkArXIHI9r2ageleaDdO1Bs4Msp19Q8FkXAkg2MeFqsfFxlLx aSQvrlO6V1kGV/P/fgrJYhY3BO2WWSQ= X-Google-Smtp-Source: ABdhPJw/5P51lScdZwvOj7VvDC9s+m9Zl7r7fHDNNT61HqTihfSyw497VFcTUYMUMWL2aPY74l4GLQ== X-Received: by 2002:a05:600c:6019:b0:38f:f021:aea4 with SMTP id az25-20020a05600c601900b0038ff021aea4mr3297469wmb.195.1650455526046; Wed, 20 Apr 2022 04:52:06 -0700 (PDT) Received: from ?IPV6:2001:8a0:f924:2600:209d:85e2:409e:8726? ([2001:8a0:f924:2600:209d:85e2:409e:8726]) by smtp.gmail.com with ESMTPSA id l28-20020a05600c1d1c00b0038ece66f1b0sm19393276wms.8.2022.04.20.04.52.04 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 20 Apr 2022 04:52:05 -0700 (PDT) Message-ID: Date: Wed, 20 Apr 2022 12:52:04 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.0 Subject: Re: [PATCH v2] gdbsupport: add path_join function Content-Language: en-US To: Simon Marchi , gdb-patches@sourceware.org Cc: Simon Marchi References: <20220420002005.226872-1-simon.marchi@polymtl.ca> From: Pedro Alves In-Reply-To: <20220420002005.226872-1-simon.marchi@polymtl.ca> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Apr 2022 11:52:09 -0000 On 2022-04-20 01:20, Simon Marchi via Gdb-patches wrote: ... > +/* Test path_join. */ > + > +static void > +test () > +{ > + test_one ("/foo/bar", "/foo", "bar"); > + test_one ("/bar", "/foo", "/bar"); > + test_one ("/bar", "/", "bar"); > + test_one ("/bar", "/", "/bar"); > + test_one ("/bar", "foo/", "/bar"); > + test_one ("foo/bar/", "foo", "bar", ""); > + test_one ("/foo", "", "/foo"); > + test_one ("foo", "", "foo"); > + test_one ("foo/bar", "foo", "", "bar"); > + test_one ("foo/", "foo", ""); > + test_one ("foo/", "foo/", ""); > + > + test_one ("D:/foo/bar", "D:/foo", "bar"); > + test_one ("D:/foo/bar", "D:/foo/", "bar"); > + > +#if defined(_WIN32) > + /* The current implementation doesn't recognize backslashes as directory > + separators on Unix-like systems, so only run these tests on Windows. If > + we ever switch our implementation to use std::filesystem::path, they > + should work anywhere, in theory. */ > + test_one ("D:\\foo/bar", "D:\\foo", "bar"); > + test_one ("D:\\foo\\bar", "D:\\foo\\", "bar"); > + test_one ("\\\\server\\dir\\file", "\\\\server\\dir\\", "file"); > + test_one ("\\\\server\\dir/file", "\\\\server\\dir", "file"); I understand we don't implement the root name checks described in 1) at: https://en.cppreference.com/w/cpp/filesystem/path/append however, at least recognizing "drive:/" and "drive:\" as absolute paths I think works as is, and I'd think is important that it works for GDB. So I think we should have these tests under _WIN32: test_one ("C:\\bar", "C:\\foo", "C:\\bar"); // '\' sep. test_one ("C:/bar", "C:\\foo", "C:/bar"); // '/' sep. test_one ("D:\\bar", "C:\\foo", "D:\\bar"); // 'D:' wins. > diff --git a/gdbsupport/gdb_string_view.h b/gdbsupport/gdb_string_view.h > index f8150288ee57..4e26bd8ff2c1 100644 > --- a/gdbsupport/gdb_string_view.h > +++ b/gdbsupport/gdb_string_view.h > @@ -34,7 +34,7 @@ > // > > > -#if __cplusplus >= 201703L > +#if __cplusplus >= 201703L && 0 > Some leftover experimentation? > > +/* Join elements in PATHS into a single path. > + > + This function is intended to have the same behavior as concatenating > + std::filesystem::path objects with std::filesystem::operator/ (which was > + introduced in C++17). */ I think it would help the reader be aware about what this means wrt to joining with absolute paths. You have that in the commit log, and I think it would be useful to have it here in a comment too: " (...) joining with an absolute path on the right hand side, like this: path_join ("/foo", "/bar"); results in just the right hand side being kept. In this example, the result is "/bar". " Otherwise this LGTM.