From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from eggs.gnu.org (eggs.gnu.org [IPv6:2001:470:142:3::10]) by sourceware.org (Postfix) with ESMTPS id 34C15385843E for ; Fri, 15 Apr 2022 05:59:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 34C15385843E Received: from fencepost.gnu.org ([2001:470:142:3::e]:52230) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nfEzR-0003P5-GV; Fri, 15 Apr 2022 01:59:30 -0400 Received: from [87.69.77.57] (port=2357 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nfEzP-0007vv-Cl; Fri, 15 Apr 2022 01:59:29 -0400 Date: Fri, 15 Apr 2022 08:59:39 +0300 Message-Id: <83h76u3n04.fsf@gnu.org> From: Eli Zaretskii To: Simon Marchi Cc: gdb-patches@sourceware.org In-Reply-To: <20220414200137.3479373-5-simon.marchi@polymtl.ca> (message from Simon Marchi via Gdb-patches on Thu, 14 Apr 2022 16:01:37 -0400) Subject: Re: [PATCH 5/5] gdbsupport: add path_join function References: <20220414200137.3479373-1-simon.marchi@polymtl.ca> <20220414200137.3479373-5-simon.marchi@polymtl.ca> X-Spam-Status: No, score=2.1 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_BARRACUDACENTRAL, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.4 X-Spam-Level: ** 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: Fri, 15 Apr 2022 05:59:33 -0000 > Date: Thu, 14 Apr 2022 16:01:37 -0400 > From: Simon Marchi via Gdb-patches > Cc: Simon Marchi > > From: Simon Marchi > > In this review [1], Eli pointed out that we should be careful when > concatenating file names to avoid duplicated slashes. On Windows, a > double slash at the beginning of a file path has a special meaning. So > naively concatenating "/" and "foo/bar" would give "//foo/bar", which > would not give the desired results. We already have a few spots doing: > > if (first_path ends with a slash) > path = first_path + second_path > else > path = first_path + slash + second_path > > In general, I think it's nice to avoid superfluous slashes in file > paths, since they might end up visible to the user and look a bit > unprofessional. > > Introduce the path_join function that can be used to join multiple path > components together (along with unit tests). Thanks. > +static void > +test () > +{ > + std::string s; > + > + s = ::path_join ("/foo", "bar", nullptr); > + SELF_CHECK (s == "/foo/bar"); > + > + s = ::path_join ("/foo", "/bar", nullptr); > + SELF_CHECK (s == "/foo/bar"); > + > + s = ::path_join ("/", "bar", nullptr); > + SELF_CHECK (s == "/bar"); > + > + s = ::path_join ("foo/", "/bar", nullptr); > + SELF_CHECK (s == "foo/bar"); > + > + s = ::path_join ("foo", "bar", "", nullptr); > + SELF_CHECK (s == "foo/bar"); > + > + s = ::path_join ("", "/foo", nullptr); > + SELF_CHECK (s == "/foo"); > + > + s = ::path_join ("", "foo", nullptr); > + SELF_CHECK (s == "foo"); > + > + s = ::path_join ("foo", "", "bar", nullptr); > + SELF_CHECK (s == "foo/bar"); > + > + s = ::path_join ("foo", "", nullptr); > + SELF_CHECK (s == "foo"); > + > + s = ::path_join ("foo/", "", nullptr); > + SELF_CHECK (s == "foo/"); Suggest to add a couple of Windows-specific tests here: one which starts with "d:/" instead of just "/", and another with backslashes. > +std::string > +path_join (const char *component...) > +{ > + std::string path = component; > + > + auto skip_leading_dir_seps = [] (const char *s) > + { > + while (*s == '/') > + ++s; > + > + return s; > + }; > + > + va_list args; > + va_start (args, component); > + > + const char *c = va_arg (args, const char *); > + while (c != nullptr) > + { > + if (!path.empty ()) > + c = skip_leading_dir_seps (c); > + > + if (*c != '\0') > + { > + if (!path.empty () && path.back () != '/') > + path += '/'; > + > + path += c; > + } > + > + c = va_arg (args, const char *); > + } > + > + va_end (args); > + > + return path; > +} I think this should use IS_DIR_SEPARATOR (or override the '=' and '!=' operators), to support Windows file names with backslashes.