Re: [PATCH 5/5] gdbsupport: add path_join function

[Simon Marchi <simon.marchi@polymtl.ca>]

>>> 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).

Well, I've been thinking about it, and perhaps that appending an
absolute path to another path (either absolute or non-absolute),
sysroot-style, is indeed a distinct use case that a regular path_join is
not meant to solve.  The code that Pedro pointed to, in solib_find_1,
convinced me.  It tries different things when dealing with a Windows
paths that contains a drive letter.

And compatibility-wise, being a drop-in replacement for
std::filesystem::operator/ is interesting.

> 
>> 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<typename ...Args>
>     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<typename ...Args>
>     inline std::string
>     path_join (Args... comps)
>     {
>       std::string res;
>       impl::path_join_1 (res, comps...);
>       return res;
>     }

Thanks for the suggestion, I ended up using Tom's proposal.

Simon