From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lndn.lancelotsix.com (lndn.lancelotsix.com [51.195.220.111]) by sourceware.org (Postfix) with ESMTPS id 330DE3858D1E for ; Mon, 18 Apr 2022 23:11:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 330DE3858D1E Received: from ubuntu.lan (unknown [IPv6:2a02:390:9086::635]) by lndn.lancelotsix.com (Postfix) with ESMTPSA id 694AC818C2; Mon, 18 Apr 2022 23:11:21 +0000 (UTC) Date: Mon, 18 Apr 2022 23:11:16 +0000 From: Lancelot SIX To: Simon Marchi Cc: gdb-patches@sourceware.org, Simon Marchi Subject: Re: [PATCH 5/5] gdbsupport: add path_join function Message-ID: <20220418231116.qhpo34hnaxpjwzcs@ubuntu.lan> References: <20220414200137.3479373-1-simon.marchi@polymtl.ca> <20220414200137.3479373-5-simon.marchi@polymtl.ca> <20220415143827.t2nlcfhmh2pondev@ubuntu.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.11 (lndn.lancelotsix.com [0.0.0.0]); Mon, 18 Apr 2022 23:11:21 +0000 (UTC) X-Spam-Status: No, score=-5.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, 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: Mon, 18 Apr 2022 23:11:24 -0000 Hi, > > Is this different semantics made on purpose? > > I indeed noticed how os.path.join handled ("/foo", "/bar"). > > I went with the behavior in my patch mostly because there are cases in > GDB where we really need to append absolute paths to another path. For > example, looking up files in sysroots. If the sysroot is set to > "/the/sysroot/" (with or without the trailing slash) and we want to load > the target library /usr/lib/libfoo.so, it would be nice if > > path_join (sysroot_path, lib_path) > > gave > > /the/sysroot/usr/lib/libfoo.so > > without a double-slash. Same when we are looking up files in the > debug-file-directory (/usr/lib/debug). > > When changing those implementations that currently use concat(...) or > string_printf("%s/%s"), we would need to be careful: if the right hand > side is an absolute path, then we would have to skip the leading > directory separator before passing it to path_join, to get the desired > result. And then we're back at having some special path handling > details everywhere. Fair enough. This is a good enough reason to have a special semantics (we could have a path_join and a sysroot_path_join, but not sure it is worth it having 2 functions). > If you can make it work using gdb::string_view or const char *, I think > it would be ok. I'll give it a try. But otherwise, I am personally > fine with the sentinel nullptr, given that the compiler gives you a > warning if you forget it. > Something like this avoids copying some std::string around (which my initial implementation did), and can work with a string view (would be the same architecture with const char*). The compiler should optimize almost all the function calls away easily enough. I saw that Tom gave another approach which would work fine as well. namespace impl { template inline void path_join_1 (std::string &l, gdb::string_view r, Args... comps) { path_join_1 (l, r); path_join_1 (l, ...comps); } template<> inline void path_join_1 (std::string &l, gdb::string_view r) { /* The real logic here */ } } /* namespace impl */ template inline std::string path_join (Args... comps) { std::string res; impl::path_join_1 (res, comps...); return res; } Best, Lancelot.