From ffaebc199ed7b1f7e539c938e8b234d2a4ad9df9 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 20 Apr 2022 17:03:25 -0400 Subject: gdbsupport: add path_join function 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). I initially wanted to make it possible to join two absolute paths, to support the use case of prepending a sysroot path to a target file path, or the prepending the debug-file-directory to a target file path. But the code in solib_find_1 shows that it is more complex than this anyway (for example, when the right hand side is a Windows path with a drive letter). So I don't think we need to support that case in path_join. That also keeps the implementation simpler. Change a few spots to use path_join to show how it can be used. I believe that all the spots I changed are guarded by some checks that ensure the right hand side operand is not an absolute path. Regression-tested on Ubuntu 18.04. Built-tested on Windows, and I also ran the new unit-test there. [1] https://sourceware.org/pipermail/gdb-patches/2022-April/187559.html Change-Id: I0df889f7e3f644e045f42ff429277b732eb6c752 --- gdbsupport/pathstuff.cc | 45 ++++++++++++++++++++++++++++++--------------- gdbsupport/pathstuff.h | 23 +++++++++++++++++++++++ 2 files changed, 53 insertions(+), 15 deletions(-) (limited to 'gdbsupport') diff --git a/gdbsupport/pathstuff.cc b/gdbsupport/pathstuff.cc index ac65651..5b5a8ee 100644 --- a/gdbsupport/pathstuff.cc +++ b/gdbsupport/pathstuff.cc @@ -119,10 +119,7 @@ gdb_realpath_keepfile (const char *filename) directory separator, avoid doubling it. */ gdb::unique_xmalloc_ptr path_storage = gdb_realpath (dir_name); const char *real_path = path_storage.get (); - if (IS_DIR_SEPARATOR (real_path[strlen (real_path) - 1])) - return string_printf ("%s%s", real_path, base_name); - else - return string_printf ("%s/%s", real_path, base_name); + return path_join (real_path, base_name); } /* See gdbsupport/pathstuff.h. */ @@ -138,12 +135,7 @@ gdb_abspath (const char *path) if (IS_ABSOLUTE_PATH (path) || current_directory == NULL) return path; - /* Beware the // my son, the Emacs barfs, the botch that catch... */ - return string_printf - ("%s%s%s", current_directory, - (IS_DIR_SEPARATOR (current_directory[strlen (current_directory) - 1]) - ? "" : SLASH_STRING), - path); + return path_join (current_directory, path); } /* See gdbsupport/pathstuff.h. */ @@ -198,6 +190,29 @@ child_path (const char *parent, const char *child) /* See gdbsupport/pathstuff.h. */ +std::string +path_join (gdb::array_view paths) +{ + std::string ret; + + for (int i = 0; i < paths.size (); ++i) + { + const gdb::string_view path = paths[i]; + + if (i > 0) + gdb_assert (!IS_ABSOLUTE_PATH (path)); + + if (!ret.empty () && !IS_DIR_SEPARATOR (ret.back ())) + ret += '/'; + + ret.append (path.begin (), path.end ()); + } + + return ret; +} + +/* See gdbsupport/pathstuff.h. */ + bool contains_dir_separator (const char *path) { @@ -227,7 +242,7 @@ get_standard_cache_dir () { /* Make sure the path is absolute and tilde-expanded. */ std::string abs = gdb_abspath (xdg_cache_home); - return string_printf ("%s/gdb", abs.c_str ()); + return path_join (abs.c_str (), "gdb"); } #endif @@ -236,7 +251,7 @@ get_standard_cache_dir () { /* Make sure the path is absolute and tilde-expanded. */ std::string abs = gdb_abspath (home); - return string_printf ("%s/" HOME_CACHE_DIR "/gdb", abs.c_str ()); + return path_join (abs.c_str (), HOME_CACHE_DIR, "gdb"); } #ifdef WIN32 @@ -245,7 +260,7 @@ get_standard_cache_dir () { /* Make sure the path is absolute and tilde-expanded. */ std::string abs = gdb_abspath (win_home); - return string_printf ("%s/gdb", abs.c_str ()); + return path_join (abs.c_str (), "gdb"); } #endif @@ -294,7 +309,7 @@ get_standard_config_dir () { /* Make sure the path is absolute and tilde-expanded. */ std::string abs = gdb_abspath (xdg_config_home); - return string_printf ("%s/gdb", abs.c_str ()); + return path_join (abs.c_str (), "gdb"); } #endif @@ -303,7 +318,7 @@ get_standard_config_dir () { /* Make sure the path is absolute and tilde-expanded. */ std::string abs = gdb_abspath (home); - return string_printf ("%s/" HOME_CONFIG_DIR "/gdb", abs.c_str ()); + return path_join (abs.c_str (), HOME_CONFIG_DIR, "gdb"); } return {}; diff --git a/gdbsupport/pathstuff.h b/gdbsupport/pathstuff.h index f6c51e9..c859279 100644 --- a/gdbsupport/pathstuff.h +++ b/gdbsupport/pathstuff.h @@ -21,6 +21,7 @@ #define COMMON_PATHSTUFF_H #include "gdbsupport/byte-vector.h" +#include "gdbsupport/array-view.h" #include #include @@ -60,6 +61,28 @@ extern std::string gdb_abspath (const char *path); extern const char *child_path (const char *parent, const char *child); +/* Join elements in PATHS into a single path. + + The first element can be absolute or relative. All the others must be + relative. */ + +extern std::string path_join (gdb::array_view paths); + +/* Same as the above, but accept paths as distinct parameters. */ + +template +std::string +path_join (Args... paths) +{ + /* It doesn't make sense to join less than two paths. */ + gdb_static_assert (sizeof... (Args) >= 2); + + std::array views + { gdb::string_view (paths)... }; + + return path_join (gdb::array_view (views)); +} + /* Return whether PATH contains a directory separator character. */ extern bool contains_dir_separator (const char *path); -- cgit v1.1