From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from barracuda.ebox.ca (barracuda.ebox.ca [96.127.255.19]) by sourceware.org (Postfix) with ESMTPS id A67863858C2C for ; Thu, 14 Apr 2022 20:01:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A67863858C2C X-ASG-Debug-ID: 1649966498-0c856e06adba2150001-fS2M51 Received: from smtp.ebox.ca (smtp.ebox.ca [96.127.255.82]) by barracuda.ebox.ca with ESMTP id yyGj8BFgpREBQvV8 (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 14 Apr 2022 16:01:38 -0400 (EDT) X-Barracuda-Envelope-From: simon.marchi@polymtl.ca X-Barracuda-RBL-Trusted-Forwarder: 96.127.255.82 Received: from simark.localdomain (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) by smtp.ebox.ca (Postfix) with ESMTP id 9D2DF441D67; Thu, 14 Apr 2022 16:01:38 -0400 (EDT) From: Simon Marchi X-Barracuda-RBL-IP: 192.222.157.6 X-Barracuda-Effective-Source-IP: 192-222-157-6.qc.cable.ebox.net[192.222.157.6] X-Barracuda-Apparent-Source-IP: 192.222.157.6 To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: [PATCH 5/5] gdbsupport: add path_join function Date: Thu, 14 Apr 2022 16:01:37 -0400 X-ASG-Orig-Subj: [PATCH 5/5] gdbsupport: add path_join function Message-Id: <20220414200137.3479373-5-simon.marchi@polymtl.ca> X-Mailer: git-send-email 2.35.2 In-Reply-To: <20220414200137.3479373-1-simon.marchi@polymtl.ca> References: <20220414200137.3479373-1-simon.marchi@polymtl.ca> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Barracuda-Connect: smtp.ebox.ca[96.127.255.82] X-Barracuda-Start-Time: 1649966498 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://96.127.255.19:443/cgi-mod/mark.cgi X-Virus-Scanned: by bsmtpd at ebox.ca X-Barracuda-Scan-Msg-Size: 15361 X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.50 X-Barracuda-Spam-Status: No, SCORE=0.50 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=8.0 tests=BSF_RULE7568M X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.97354 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 0.50 BSF_RULE7568M Custom Rule 7568M X-Spam-Status: No, score=-3612.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_QUARANTINE, KAM_DMARC_STATUS, KAM_SHORT, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_SOFTFAIL, 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: Thu, 14 Apr 2022 20:01:51 -0000 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). Change a few spots to use path_join to show how it can be used. [1] https://sourceware.org/pipermail/gdb-patches/2022-April/187559.html Change-Id: I0df889f7e3f644e045f42ff429277b732eb6c752 --- gdb/Makefile.in | 1 + gdb/buildsym.c | 5 +- gdb/dwarf2/line-header.c | 3 +- gdb/dwarf2/read.c | 43 ++++++++--------- gdb/macrotab.c | 3 +- gdb/unittests/path-join-selftests.c | 75 +++++++++++++++++++++++++++++ gdbsupport/pathstuff.cc | 62 ++++++++++++++++++------ gdbsupport/pathstuff.h | 11 +++++ 8 files changed, 161 insertions(+), 42 deletions(-) create mode 100644 gdb/unittests/path-join-selftests.c diff --git a/gdb/Makefile.in b/gdb/Makefile.in index c8e140b55449..ec0e55dd803c 100644 --- a/gdb/Makefile.in +++ b/gdb/Makefile.in @@ -465,6 +465,7 @@ SELFTESTS_SRCS = \ unittests/optional-selftests.c \ unittests/parallel-for-selftests.c \ unittests/parse-connection-spec-selftests.c \ + unittests/path-join-selftests.c \ unittests/ptid-selftests.c \ unittests/main-thread-selftests.c \ unittests/mkdir-recursive-selftests.c \ diff --git a/gdb/buildsym.c b/gdb/buildsym.c index 628903d674f4..a1375e84a4ac 100644 --- a/gdb/buildsym.c +++ b/gdb/buildsym.c @@ -20,6 +20,7 @@ #include "buildsym-legacy.h" #include "bfd.h" #include "gdbsupport/gdb_obstack.h" +#include "gdbsupport/pathstuff.h" #include "symtab.h" #include "symfile.h" #include "objfiles.h" @@ -507,8 +508,8 @@ buildsym_compunit::start_subfile (const char *name) && !IS_ABSOLUTE_PATH (subfile->name) && !m_comp_dir.empty ()) { - subfile_name_holder = string_printf ("%s/%s", m_comp_dir.c_str (), - subfile->name.c_str ()); + subfile_name_holder = path_join (m_comp_dir.c_str (), + subfile->name.c_str (), nullptr); subfile_name = subfile_name_holder.c_str (); } else diff --git a/gdb/dwarf2/line-header.c b/gdb/dwarf2/line-header.c index 77e7e553b21e..9f4447023c58 100644 --- a/gdb/dwarf2/line-header.c +++ b/gdb/dwarf2/line-header.c @@ -24,6 +24,7 @@ #include "dwarf2/read.h" #include "complaints.h" #include "filenames.h" +#include "gdbsupport/pathstuff.h" void line_header::add_include_dir (const char *include_dir) @@ -73,7 +74,7 @@ line_header::file_file_name (int file) const { const char *dir = fe->include_dir (this); if (dir != NULL) - return string_printf ("%s/%s", dir, fe->name); + return path_join (dir, fe->name, nullptr); } return fe->name; diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 6dcd446e5f46..508cb511b4a8 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -1343,7 +1343,7 @@ static const char *compute_include_file_name (const struct line_header *lh, const file_entry &fe, const file_and_directory &cu_info, - gdb::unique_xmalloc_ptr *name_holder); + std::string &name_holder); static htab_up allocate_signatured_type_table (); @@ -2809,9 +2809,9 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader, { for (const auto &entry : lh->file_names ()) { - gdb::unique_xmalloc_ptr name_holder; + std::string name_holder; const char *include_name = - compute_include_file_name (lh.get (), entry, fnd, &name_holder); + compute_include_file_name (lh.get (), entry, fnd, name_holder); if (include_name != nullptr) { include_name = per_objfile->objfile->intern (include_name); @@ -11166,14 +11166,13 @@ open_dwo_file (dwarf2_per_objfile *per_objfile, if (comp_dir != NULL) { - gdb::unique_xmalloc_ptr path_to_try - (concat (comp_dir, SLASH_STRING, file_name, (char *) NULL)); + std::string path_to_try = path_join (comp_dir, file_name, nullptr); /* NOTE: If comp_dir is a relative path, this will also try the search path, which seems useful. */ - gdb_bfd_ref_ptr abfd (try_open_dwop_file (per_objfile, path_to_try.get (), - 0 /*is_dwp*/, - 1 /*search_cwd*/)); + gdb_bfd_ref_ptr abfd (try_open_dwop_file + (per_objfile, path_to_try.c_str (), 0 /*is_dwp*/, 1 /*search_cwd*/)); + if (abfd != NULL) return abfd; } @@ -19751,14 +19750,14 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu) static const char * compute_include_file_name (const struct line_header *lh, const file_entry &fe, const file_and_directory &cu_info, - gdb::unique_xmalloc_ptr *name_holder) + std::string &name_holder) { const char *include_name = fe.name; const char *include_name_to_compare = include_name; const char *dir_name = fe.include_dir (lh); - gdb::unique_xmalloc_ptr hold_compare; + std::string hold_compare; if (!IS_ABSOLUTE_PATH (include_name) && (dir_name != nullptr || cu_info.get_comp_dir () != nullptr)) { @@ -19785,27 +19784,25 @@ compute_include_file_name (const struct line_header *lh, const file_entry &fe, if (dir_name != NULL) { - name_holder->reset (concat (dir_name, SLASH_STRING, - include_name, (char *) NULL)); - include_name = name_holder->get (); + name_holder = path_join (dir_name, include_name, nullptr); + include_name = name_holder.c_str (); include_name_to_compare = include_name; } if (!IS_ABSOLUTE_PATH (include_name) && cu_info.get_comp_dir () != nullptr) { - hold_compare.reset (concat (cu_info.get_comp_dir (), SLASH_STRING, - include_name, (char *) NULL)); - include_name_to_compare = hold_compare.get (); + hold_compare = path_join (cu_info.get_comp_dir (), include_name, + nullptr); + include_name_to_compare = hold_compare.c_str (); } } - gdb::unique_xmalloc_ptr copied_name; + std::string copied_name; const char *cu_filename = cu_info.get_name (); if (!IS_ABSOLUTE_PATH (cu_filename) && cu_info.get_comp_dir () != nullptr) { - copied_name.reset (concat (cu_info.get_comp_dir (), SLASH_STRING, - cu_filename, (char *) NULL)); - cu_filename = copied_name.get (); + copied_name = path_join (cu_info.get_comp_dir (), cu_filename, nullptr); + cu_filename = copied_name.c_str (); } if (FILENAME_CMP (include_name_to_compare, cu_filename) == 0) @@ -20540,7 +20537,7 @@ static void dwarf2_start_subfile (struct dwarf2_cu *cu, const char *filename, const char *dirname) { - gdb::unique_xmalloc_ptr copy; + std::string copy; /* In order not to lose the line information directory, we concatenate it to the filename when it makes sense. @@ -20551,8 +20548,8 @@ dwarf2_start_subfile (struct dwarf2_cu *cu, const char *filename, if (!IS_ABSOLUTE_PATH (filename) && dirname != NULL) { - copy.reset (concat (dirname, SLASH_STRING, filename, (char *) NULL)); - filename = copy.get (); + copy = path_join (dirname, filename, nullptr); + filename = copy.c_str (); } cu->get_builder ()->start_subfile (filename); diff --git a/gdb/macrotab.c b/gdb/macrotab.c index 7ca3f312d330..b893c7ae3c1b 100644 --- a/gdb/macrotab.c +++ b/gdb/macrotab.c @@ -19,6 +19,7 @@ #include "defs.h" #include "gdbsupport/gdb_obstack.h" +#include "gdbsupport/pathstuff.h" #include "splay-tree.h" #include "filenames.h" #include "symtab.h" @@ -1071,5 +1072,5 @@ macro_source_fullname (struct macro_source_file *file) if (comp_dir == NULL || IS_ABSOLUTE_PATH (file->filename)) return file->filename; - return std::string (comp_dir) + SLASH_STRING + file->filename; + return path_join (comp_dir, file->filename, nullptr); } diff --git a/gdb/unittests/path-join-selftests.c b/gdb/unittests/path-join-selftests.c new file mode 100644 index 000000000000..f0dfdfd40376 --- /dev/null +++ b/gdb/unittests/path-join-selftests.c @@ -0,0 +1,75 @@ +/* Self tests for path_join for GDB, the GNU debugger. + + Copyright (C) 2022 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#include "defs.h" +#include "gdbsupport/pathstuff.h" +#include "gdbsupport/selftest.h" + +namespace selftests { +namespace path_join { + +/* Test path_join. */ + +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/"); +} + +} +} + +void _initialize_path_join_selftests (); +void +_initialize_path_join_selftests () +{ + selftests::register_test ("path_join", + selftests::path_join::test); +} + diff --git a/gdbsupport/pathstuff.cc b/gdbsupport/pathstuff.cc index ac65651d4492..2d95eaff3d16 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, nullptr); } /* 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, nullptr); } /* See gdbsupport/pathstuff.h. */ @@ -198,6 +190,46 @@ child_path (const char *parent, const char *child) /* See gdbsupport/pathstuff.h. */ +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; +} + +/* See gdbsupport/pathstuff.h. */ + bool contains_dir_separator (const char *path) { @@ -227,7 +259,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", nullptr); } #endif @@ -236,7 +268,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", nullptr); } #ifdef WIN32 @@ -245,7 +277,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", nullptr); } #endif @@ -294,7 +326,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", nullptr); } #endif @@ -303,7 +335,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", nullptr); } return {}; diff --git a/gdbsupport/pathstuff.h b/gdbsupport/pathstuff.h index f6c51e9de887..3e776be072e6 100644 --- a/gdbsupport/pathstuff.h +++ b/gdbsupport/pathstuff.h @@ -60,6 +60,17 @@ extern std::string gdb_abspath (const char *path); extern const char *child_path (const char *parent, const char *child); +/* Join COMPONENT and all subsequent null-terminated string arguments into a + single path. + + The last argument must be nullptr to denote the end of the argument list + + Empty arguments or arguments consisting only of directory separators are + ignored. */ + +extern std::string path_join (const char *component, ...) + ATTRIBUTE_SENTINEL; + /* Return whether PATH contains a directory separator character. */ extern bool contains_dir_separator (const char *path); -- 2.35.2