From 079786775899c16e1ef96731f3ada9d42b41b544 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20St=C3=B6neberg?= Date: Thu, 23 Feb 2023 22:34:05 +0100 Subject: [PATCH] improved `Path` handling of mixed separators (#4808) --- lib/path.cpp | 30 ++++++++------ lib/path.h | 4 +- lib/platform.cpp | 14 ++++--- lib/symboldatabase.cpp | 2 +- lib/valueflow.cpp | 2 +- test/testpath.cpp | 88 ++++++++++++++++++++++++++++++++++++------ 6 files changed, 107 insertions(+), 33 deletions(-) diff --git a/lib/path.cpp b/lib/path.cpp index 86fbd814c..b972c383a 100644 --- a/lib/path.cpp +++ b/lib/path.cpp @@ -32,6 +32,7 @@ #include #else #include +#include #endif #if defined(__CYGWIN__) #include @@ -150,9 +151,12 @@ bool Path::isAbsolute(const std::string& path) return false; } -std::string Path::getRelativePath(const std::string& absolutePath, const std::vector& basePaths) +std::string Path::getRelativePath(std::string absolutePath, const std::vector& basePaths) { - for (const std::string &bp : basePaths) { + absolutePath = Path::fromNativeSeparators(std::move(absolutePath)); + + for (std::string bp : basePaths) { + bp = Path::fromNativeSeparators(std::move(bp)); if (absolutePath == bp || bp.empty()) // Seems to be a file, or path is empty continue; @@ -161,7 +165,7 @@ std::string Path::getRelativePath(const std::string& absolutePath, const std::ve if (endsWith(bp,'/')) return absolutePath.substr(bp.length()); - else if (absolutePath.size() > bp.size() && absolutePath[bp.length()] == '/') + if (absolutePath.size() > bp.size() && absolutePath[bp.length()] == '/') return absolutePath.substr(bp.length() + 1); } return absolutePath; @@ -211,9 +215,13 @@ std::string Path::getAbsoluteFilePath(const std::string& filePath) if (_fullpath(absolute, filePath.c_str(), _MAX_PATH)) absolute_path = absolute; #elif defined(__linux__) || defined(__sun) || defined(__hpux) || defined(__GNUC__) || defined(__CPPCHECK__) + // realpath() only works with files that actually exist char * absolute = realpath(filePath.c_str(), nullptr); - if (absolute) - absolute_path = absolute; + if (!absolute) { + const int err = errno; + throw std::runtime_error("realpath() failed with " + std::to_string(err)); + } + absolute_path = absolute; free(absolute); #else #error Platform absolute path function needed @@ -221,15 +229,11 @@ std::string Path::getAbsoluteFilePath(const std::string& filePath) return absolute_path; } -std::string Path::stripDirectoryPart(const std::string &file) +std::string Path::stripDirectoryPart(std::string file) { -#if defined(_WIN32) && !defined(__MINGW32__) - const char native = '\\'; -#else - const char native = '/'; -#endif + file = fromNativeSeparators(std::move(file)); - const std::string::size_type p = file.rfind(native); + const std::string::size_type p = file.rfind('/'); if (p != std::string::npos) { return file.substr(p + 1); } @@ -243,6 +247,8 @@ bool Path::fileExists(const std::string &file) } std::string Path::join(std::string path1, std::string path2) { + path1 = fromNativeSeparators(std::move(path1)); + path2 = fromNativeSeparators(std::move(path2)); if (path1.empty() || path2.empty()) return path1 + path2; if (path2.front() == '/') diff --git a/lib/path.h b/lib/path.h index 4d17a593a..3898c79df 100644 --- a/lib/path.h +++ b/lib/path.h @@ -117,7 +117,7 @@ public: * @param basePaths Paths to which it may be made relative. * @return relative path, if possible. Otherwise absolutePath is returned unchanged */ - static std::string getRelativePath(const std::string& absolutePath, const std::vector& basePaths); + static std::string getRelativePath(std::string absolutePath, const std::vector& basePaths); /** * @brief Get an absolute file path from a relative one. @@ -172,7 +172,7 @@ public: * @param file filename to be stripped. path info is optional * @return filename without directory path part. */ - static std::string stripDirectoryPart(const std::string &file); + static std::string stripDirectoryPart(std::string file); /** * @brief Checks if a File exists diff --git a/lib/platform.cpp b/lib/platform.cpp index 4794a3758..8f1e5006d 100644 --- a/lib/platform.cpp +++ b/lib/platform.cpp @@ -178,6 +178,8 @@ bool cppcheck::Platform::platform(const std::string& platformstr, std::string& e return false; } else { + if (verbose) + std::cout << "current working directory '" + Path::getCurrentPath() + "'" << std::endl; bool found = false; for (const std::string& path : paths) { if (verbose) @@ -203,19 +205,19 @@ bool cppcheck::Platform::loadPlatformFile(const char exename[], const std::strin std::vector filenames; filenames.push_back(filename); filenames.push_back(filename + ".xml"); - filenames.push_back("platforms/" + filename); - filenames.push_back("platforms/" + filename + ".xml"); + filenames.push_back(Path::join("platforms", filename)); + filenames.push_back(Path::join("platforms", filename + ".xml")); if (exename && (std::string::npos != Path::fromNativeSeparators(exename).find('/'))) { filenames.push_back(Path::getPathFromFilename(Path::fromNativeSeparators(exename)) + filename); - filenames.push_back(Path::getPathFromFilename(Path::fromNativeSeparators(exename)) + "platforms/" + filename); - filenames.push_back(Path::getPathFromFilename(Path::fromNativeSeparators(exename)) + "platforms/" + filename + ".xml"); + filenames.push_back(Path::getPathFromFilename(Path::fromNativeSeparators(exename)) + Path::join("platforms", filename)); + filenames.push_back(Path::getPathFromFilename(Path::fromNativeSeparators(exename)) + Path::join("platforms", filename + ".xml")); } #ifdef FILESDIR std::string filesdir = FILESDIR; if (!filesdir.empty() && filesdir[filesdir.size()-1] != '/') filesdir += '/'; - filenames.push_back(filesdir + ("platforms/" + filename)); - filenames.push_back(filesdir + ("platforms/" + filename + ".xml")); + filenames.push_back(filesdir + Path::join("platforms", filename)); + filenames.push_back(filesdir + Path::join("platforms", filename + ".xml")); #endif // open file.. diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 352de2c66..baa87a36f 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -7474,7 +7474,7 @@ void ValueType::setDebugPath(const Token* tok, SourceLocation ctx, SourceLocatio std::string file = ctx.file_name(); if (file.empty()) return; - std::string s = Path::stripDirectoryPart(file) + ":" + MathLib::toString(ctx.line()) + ": " + ctx.function_name() + + std::string s = Path::stripDirectoryPart(std::move(file)) + ":" + MathLib::toString(ctx.line()) + ": " + ctx.function_name() + " => " + local.function_name(); debugPath.emplace_back(tok, std::move(s)); } diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index ceed40ad4..a826d92a2 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -168,7 +168,7 @@ static void setSourceLocation(ValueFlow::Value& v, std::string file = ctx.file_name(); if (file.empty()) return; - std::string s = Path::stripDirectoryPart(file) + ":" + MathLib::toString(ctx.line()) + ": " + ctx.function_name() + + std::string s = Path::stripDirectoryPart(std::move(file)) + ":" + MathLib::toString(ctx.line()) + ": " + ctx.function_name() + " => " + local.function_name() + ": " + debugString(v); v.debugPath.emplace_back(tok, std::move(s)); } diff --git a/test/testpath.cpp b/test/testpath.cpp index 1cf8f242c..81e23cc44 100644 --- a/test/testpath.cpp +++ b/test/testpath.cpp @@ -38,6 +38,8 @@ private: TEST_CASE(is_cpp); TEST_CASE(get_path_from_filename); TEST_CASE(join); + TEST_CASE(getAbsoluteFilePath); + TEST_CASE(stripDirectoryPart); } void removeQuotationMarks() const { @@ -93,18 +95,48 @@ private: } void getRelative() const { - const std::vector basePaths = { - "", // Don't crash with empty paths - "C:/foo", - "C:/bar/", - "C:/test.cpp" - }; + { + const std::vector basePaths = { + "", // Don't crash with empty paths + "C:/foo", + "C:/bar/", + "C:/test.cpp" + }; - ASSERT_EQUALS("x.c", Path::getRelativePath("C:/foo/x.c", basePaths)); - ASSERT_EQUALS("y.c", Path::getRelativePath("C:/bar/y.c", basePaths)); - ASSERT_EQUALS("foo/y.c", Path::getRelativePath("C:/bar/foo/y.c", basePaths)); - ASSERT_EQUALS("C:/test.cpp", Path::getRelativePath("C:/test.cpp", basePaths)); - ASSERT_EQUALS("C:/foobar/test.cpp", Path::getRelativePath("C:/foobar/test.cpp", basePaths)); + ASSERT_EQUALS("x.c", Path::getRelativePath("C:/foo/x.c", basePaths)); + ASSERT_EQUALS("y.c", Path::getRelativePath("C:/bar/y.c", basePaths)); + ASSERT_EQUALS("foo/y.c", Path::getRelativePath("C:/bar/foo/y.c", basePaths)); + ASSERT_EQUALS("C:/test.cpp", Path::getRelativePath("C:/test.cpp", basePaths)); + ASSERT_EQUALS("C:/foobar/test.cpp", Path::getRelativePath("C:/foobar/test.cpp", basePaths)); + } + { + const std::vector basePaths = { + "", // Don't crash with empty paths + "C:\\foo", + "C:\\bar\\", + "C:\\test.cpp" + }; + + ASSERT_EQUALS("x.c", Path::getRelativePath("C:\\foo\\x.c", basePaths)); + ASSERT_EQUALS("y.c", Path::getRelativePath("C:\\bar\\y.c", basePaths)); + ASSERT_EQUALS("foo/y.c", Path::getRelativePath("C:\\bar\\foo\\y.c", basePaths)); + ASSERT_EQUALS("C:/test.cpp", Path::getRelativePath("C:\\test.cpp", basePaths)); + ASSERT_EQUALS("C:/foobar/test.cpp", Path::getRelativePath("C:\\foobar\\test.cpp", basePaths)); + } + { + const std::vector basePaths = { + "", // Don't crash with empty paths + "/c/foo", + "/c/bar/", + "/c/test.cpp" + }; + + ASSERT_EQUALS("x.c", Path::getRelativePath("/c/foo/x.c", basePaths)); + ASSERT_EQUALS("y.c", Path::getRelativePath("/c/bar/y.c", basePaths)); + ASSERT_EQUALS("foo/y.c", Path::getRelativePath("/c/bar/foo\\y.c", basePaths)); + ASSERT_EQUALS("/c/test.cpp", Path::getRelativePath("/c/test.cpp", basePaths)); + ASSERT_EQUALS("/c/foobar/test.cpp", Path::getRelativePath("/c/foobar/test.cpp", basePaths)); + } } void is_c() const { @@ -141,6 +173,12 @@ private: ASSERT_EQUALS("/tmp/", Path::getPathFromFilename("/tmp/index.h")); ASSERT_EQUALS("a/b/c/", Path::getPathFromFilename("a/b/c/index.h")); ASSERT_EQUALS("a/b/c/", Path::getPathFromFilename("a/b/c/")); + ASSERT_EQUALS("S:\\tmp\\", Path::getPathFromFilename("S:\\tmp\\index.h")); + ASSERT_EQUALS("a\\b\\c\\", Path::getPathFromFilename("a\\b\\c\\index.h")); + ASSERT_EQUALS("a\\b\\c\\", Path::getPathFromFilename("a\\b\\c\\")); + ASSERT_EQUALS("S:\\a\\b\\c\\", Path::getPathFromFilename("S:\\a\\b\\c\\")); + ASSERT_EQUALS("S:/tmp/", Path::getPathFromFilename("S:/tmp/index.h")); + ASSERT_EQUALS("S:/a/b/c/", Path::getPathFromFilename("S:/a/b/c/index.h")); } void join() const { @@ -148,7 +186,35 @@ private: ASSERT_EQUALS("a", Path::join("", "a")); ASSERT_EQUALS("a/b", Path::join("a", "b")); ASSERT_EQUALS("a/b", Path::join("a/", "b")); + ASSERT_EQUALS("a/b", Path::join("a\\", "b")); ASSERT_EQUALS("/b", Path::join("a", "/b")); + ASSERT_EQUALS("/b", Path::join("a", "\\b")); + } + + // TODO: this is quite messy - should Path::getAbsoluteFilePath() return normalized separators? + void getAbsoluteFilePath() const { + // Path::getAbsoluteFilePath() only works with existing paths on Linux +#ifdef _WIN32 + const std::string cwd = Path::getCurrentPath(); + ASSERT_EQUALS(Path::join(cwd, "a.h"), Path::fromNativeSeparators(Path::getAbsoluteFilePath("a.h"))); + ASSERT_EQUALS(Path::join(cwd, "inc/a.h"), Path::fromNativeSeparators(Path::getAbsoluteFilePath("inc/a.h"))); + const std::string cwd_down = Path::getPathFromFilename(cwd); + ASSERT_EQUALS(Path::join(cwd_down, "a.h"), Path::fromNativeSeparators(Path::getAbsoluteFilePath("../a.h"))); + ASSERT_EQUALS(Path::join(cwd_down, "inc/a.h"), Path::fromNativeSeparators(Path::getAbsoluteFilePath("../inc/a.h"))); + ASSERT_EQUALS(Path::join(cwd_down, "inc/a.h"), Path::fromNativeSeparators(Path::getAbsoluteFilePath("../inc/../inc/a.h"))); +#endif + } + + void stripDirectoryPart() const { + ASSERT_EQUALS("a.h", Path::stripDirectoryPart("a.h")); + ASSERT_EQUALS("a.h", Path::stripDirectoryPart("a/a.h")); + ASSERT_EQUALS("a.h", Path::stripDirectoryPart("a/b/a.h")); + ASSERT_EQUALS("a.h", Path::stripDirectoryPart("/mnt/a/b/a.h")); + ASSERT_EQUALS("a.h", Path::stripDirectoryPart("a\\a.h")); + ASSERT_EQUALS("a.h", Path::stripDirectoryPart("a\\b\\a.h")); + ASSERT_EQUALS("a.h", Path::stripDirectoryPart("S:\\a\\b\\a.h")); + ASSERT_EQUALS("a.h", Path::stripDirectoryPart("S:/a/b/a.h")); + } };