Skip to content

Commit 0797867

Browse files
firewavefirewave
authored
improved Path handling of mixed separators (cppcheck-opensource#4808)
1 parent 346ecdb commit 0797867

6 files changed

Lines changed: 108 additions & 34 deletions

File tree

‎lib/path.cpp‎

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#include <unistd.h>
3333
#else
3434
#include <direct.h>
35+
#include <stdexcept>
3536
#endif
3637
#if defined(__CYGWIN__)
3738
#include <strings.h>
@@ -150,9 +151,12 @@ bool Path::isAbsolute(const std::string& path)
150151
return false;
151152
}
152153

153-
std::string Path::getRelativePath(const std::string& absolutePath, const std::vector<std::string>& basePaths)
154+
std::string Path::getRelativePath(std::string absolutePath, const std::vector<std::string>& basePaths)
154155
{
155-
for (const std::string &bp : basePaths) {
156+
absolutePath = Path::fromNativeSeparators(std::move(absolutePath));
157+
158+
for (std::string bp : basePaths) {
159+
bp = Path::fromNativeSeparators(std::move(bp));
156160
if (absolutePath == bp || bp.empty()) // Seems to be a file, or path is empty
157161
continue;
158162

@@ -161,7 +165,7 @@ std::string Path::getRelativePath(const std::string& absolutePath, const std::ve
161165

162166
if (endsWith(bp,'/'))
163167
return absolutePath.substr(bp.length());
164-
else if (absolutePath.size() > bp.size() && absolutePath[bp.length()] == '/')
168+
if (absolutePath.size() > bp.size() && absolutePath[bp.length()] == '/')
165169
return absolutePath.substr(bp.length() + 1);
166170
}
167171
return absolutePath;
@@ -211,25 +215,25 @@ std::string Path::getAbsoluteFilePath(const std::string& filePath)
211215
if (_fullpath(absolute, filePath.c_str(), _MAX_PATH))
212216
absolute_path = absolute;
213217
#elif defined(__linux__) || defined(__sun) || defined(__hpux) || defined(__GNUC__) || defined(__CPPCHECK__)
218+
// realpath() only works with files that actually exist
214219
char * absolute = realpath(filePath.c_str(), nullptr);
215-
if (absolute)
216-
absolute_path = absolute;
220+
if (!absolute) {
221+
const int err = errno;
222+
throw std::runtime_error("realpath() failed with " + std::to_string(err));
223+
}
224+
absolute_path = absolute;
217225
free(absolute);
218226
#else
219227
#error Platform absolute path function needed
220228
#endif
221229
return absolute_path;
222230
}
223231

224-
std::string Path::stripDirectoryPart(const std::string &file)
232+
std::string Path::stripDirectoryPart(std::string file)
225233
{
226-
#if defined(_WIN32) && !defined(__MINGW32__)
227-
const char native = '\\';
228-
#else
229-
const char native = '/';
230-
#endif
234+
file = fromNativeSeparators(std::move(file));
231235

232-
const std::string::size_type p = file.rfind(native);
236+
const std::string::size_type p = file.rfind('/');
233237
if (p != std::string::npos) {
234238
return file.substr(p + 1);
235239
}
@@ -243,6 +247,8 @@ bool Path::fileExists(const std::string &file)
243247
}
244248

245249
std::string Path::join(std::string path1, std::string path2) {
250+
path1 = fromNativeSeparators(std::move(path1));
251+
path2 = fromNativeSeparators(std::move(path2));
246252
if (path1.empty() || path2.empty())
247253
return path1 + path2;
248254
if (path2.front() == '/')

‎lib/path.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ class CPPCHECKLIB Path {
117117
* @param basePaths Paths to which it may be made relative.
118118
* @return relative path, if possible. Otherwise absolutePath is returned unchanged
119119
*/
120-
static std::string getRelativePath(const std::string& absolutePath, const std::vector<std::string>& basePaths);
120+
static std::string getRelativePath(std::string absolutePath, const std::vector<std::string>& basePaths);
121121

122122
/**
123123
* @brief Get an absolute file path from a relative one.
@@ -172,7 +172,7 @@ class CPPCHECKLIB Path {
172172
* @param file filename to be stripped. path info is optional
173173
* @return filename without directory path part.
174174
*/
175-
static std::string stripDirectoryPart(const std::string &file);
175+
static std::string stripDirectoryPart(std::string file);
176176

177177
/**
178178
* @brief Checks if a File exists

‎lib/platform.cpp‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,8 @@ bool cppcheck::Platform::platform(const std::string& platformstr, std::string& e
178178
return false;
179179
}
180180
else {
181+
if (verbose)
182+
std::cout << "current working directory '" + Path::getCurrentPath() + "'" << std::endl;
181183
bool found = false;
182184
for (const std::string& path : paths) {
183185
if (verbose)
@@ -203,19 +205,19 @@ bool cppcheck::Platform::loadPlatformFile(const char exename[], const std::strin
203205
std::vector<std::string> filenames;
204206
filenames.push_back(filename);
205207
filenames.push_back(filename + ".xml");
206-
filenames.push_back("platforms/" + filename);
207-
filenames.push_back("platforms/" + filename + ".xml");
208+
filenames.push_back(Path::join("platforms", filename));
209+
filenames.push_back(Path::join("platforms", filename + ".xml"));
208210
if (exename && (std::string::npos != Path::fromNativeSeparators(exename).find('/'))) {
209211
filenames.push_back(Path::getPathFromFilename(Path::fromNativeSeparators(exename)) + filename);
210-
filenames.push_back(Path::getPathFromFilename(Path::fromNativeSeparators(exename)) + "platforms/" + filename);
211-
filenames.push_back(Path::getPathFromFilename(Path::fromNativeSeparators(exename)) + "platforms/" + filename + ".xml");
212+
filenames.push_back(Path::getPathFromFilename(Path::fromNativeSeparators(exename)) + Path::join("platforms", filename));
213+
filenames.push_back(Path::getPathFromFilename(Path::fromNativeSeparators(exename)) + Path::join("platforms", filename + ".xml"));
212214
}
213215
#ifdef FILESDIR
214216
std::string filesdir = FILESDIR;
215217
if (!filesdir.empty() && filesdir[filesdir.size()-1] != '/')
216218
filesdir += '/';
217-
filenames.push_back(filesdir + ("platforms/" + filename));
218-
filenames.push_back(filesdir + ("platforms/" + filename + ".xml"));
219+
filenames.push_back(filesdir + Path::join("platforms", filename));
220+
filenames.push_back(filesdir + Path::join("platforms", filename + ".xml"));
219221
#endif
220222

221223
// open file..

‎lib/symboldatabase.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7474,7 +7474,7 @@ void ValueType::setDebugPath(const Token* tok, SourceLocation ctx, SourceLocatio
74747474
std::string file = ctx.file_name();
74757475
if (file.empty())
74767476
return;
7477-
std::string s = Path::stripDirectoryPart(file) + ":" + MathLib::toString(ctx.line()) + ": " + ctx.function_name() +
7477+
std::string s = Path::stripDirectoryPart(std::move(file)) + ":" + MathLib::toString(ctx.line()) + ": " + ctx.function_name() +
74787478
" => " + local.function_name();
74797479
debugPath.emplace_back(tok, std::move(s));
74807480
}

‎lib/valueflow.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ static void setSourceLocation(ValueFlow::Value& v,
168168
std::string file = ctx.file_name();
169169
if (file.empty())
170170
return;
171-
std::string s = Path::stripDirectoryPart(file) + ":" + MathLib::toString(ctx.line()) + ": " + ctx.function_name() +
171+
std::string s = Path::stripDirectoryPart(std::move(file)) + ":" + MathLib::toString(ctx.line()) + ": " + ctx.function_name() +
172172
" => " + local.function_name() + ": " + debugString(v);
173173
v.debugPath.emplace_back(tok, std::move(s));
174174
}

‎test/testpath.cpp‎

Lines changed: 78 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ class TestPath : public TestFixture {
3838
TEST_CASE(is_cpp);
3939
TEST_CASE(get_path_from_filename);
4040
TEST_CASE(join);
41+
TEST_CASE(getAbsoluteFilePath);
42+
TEST_CASE(stripDirectoryPart);
4143
}
4244

4345
void removeQuotationMarks() const {
@@ -93,18 +95,48 @@ class TestPath : public TestFixture {
9395
}
9496

9597
void getRelative() const {
96-
const std::vector<std::string> basePaths = {
97-
"", // Don't crash with empty paths
98-
"C:/foo",
99-
"C:/bar/",
100-
"C:/test.cpp"
101-
};
102-
103-
ASSERT_EQUALS("x.c", Path::getRelativePath("C:/foo/x.c", basePaths));
104-
ASSERT_EQUALS("y.c", Path::getRelativePath("C:/bar/y.c", basePaths));
105-
ASSERT_EQUALS("foo/y.c", Path::getRelativePath("C:/bar/foo/y.c", basePaths));
106-
ASSERT_EQUALS("C:/test.cpp", Path::getRelativePath("C:/test.cpp", basePaths));
107-
ASSERT_EQUALS("C:/foobar/test.cpp", Path::getRelativePath("C:/foobar/test.cpp", basePaths));
98+
{
99+
const std::vector<std::string> basePaths = {
100+
"", // Don't crash with empty paths
101+
"C:/foo",
102+
"C:/bar/",
103+
"C:/test.cpp"
104+
};
105+
106+
ASSERT_EQUALS("x.c", Path::getRelativePath("C:/foo/x.c", basePaths));
107+
ASSERT_EQUALS("y.c", Path::getRelativePath("C:/bar/y.c", basePaths));
108+
ASSERT_EQUALS("foo/y.c", Path::getRelativePath("C:/bar/foo/y.c", basePaths));
109+
ASSERT_EQUALS("C:/test.cpp", Path::getRelativePath("C:/test.cpp", basePaths));
110+
ASSERT_EQUALS("C:/foobar/test.cpp", Path::getRelativePath("C:/foobar/test.cpp", basePaths));
111+
}
112+
{
113+
const std::vector<std::string> basePaths = {
114+
"", // Don't crash with empty paths
115+
"C:\\foo",
116+
"C:\\bar\\",
117+
"C:\\test.cpp"
118+
};
119+
120+
ASSERT_EQUALS("x.c", Path::getRelativePath("C:\\foo\\x.c", basePaths));
121+
ASSERT_EQUALS("y.c", Path::getRelativePath("C:\\bar\\y.c", basePaths));
122+
ASSERT_EQUALS("foo/y.c", Path::getRelativePath("C:\\bar\\foo\\y.c", basePaths));
123+
ASSERT_EQUALS("C:/test.cpp", Path::getRelativePath("C:\\test.cpp", basePaths));
124+
ASSERT_EQUALS("C:/foobar/test.cpp", Path::getRelativePath("C:\\foobar\\test.cpp", basePaths));
125+
}
126+
{
127+
const std::vector<std::string> basePaths = {
128+
"", // Don't crash with empty paths
129+
"/c/foo",
130+
"/c/bar/",
131+
"/c/test.cpp"
132+
};
133+
134+
ASSERT_EQUALS("x.c", Path::getRelativePath("/c/foo/x.c", basePaths));
135+
ASSERT_EQUALS("y.c", Path::getRelativePath("/c/bar/y.c", basePaths));
136+
ASSERT_EQUALS("foo/y.c", Path::getRelativePath("/c/bar/foo\\y.c", basePaths));
137+
ASSERT_EQUALS("/c/test.cpp", Path::getRelativePath("/c/test.cpp", basePaths));
138+
ASSERT_EQUALS("/c/foobar/test.cpp", Path::getRelativePath("/c/foobar/test.cpp", basePaths));
139+
}
108140
}
109141

110142
void is_c() const {
@@ -141,14 +173,48 @@ class TestPath : public TestFixture {
141173
ASSERT_EQUALS("/tmp/", Path::getPathFromFilename("/tmp/index.h"));
142174
ASSERT_EQUALS("a/b/c/", Path::getPathFromFilename("a/b/c/index.h"));
143175
ASSERT_EQUALS("a/b/c/", Path::getPathFromFilename("a/b/c/"));
176+
ASSERT_EQUALS("S:\\tmp\\", Path::getPathFromFilename("S:\\tmp\\index.h"));
177+
ASSERT_EQUALS("a\\b\\c\\", Path::getPathFromFilename("a\\b\\c\\index.h"));
178+
ASSERT_EQUALS("a\\b\\c\\", Path::getPathFromFilename("a\\b\\c\\"));
179+
ASSERT_EQUALS("S:\\a\\b\\c\\", Path::getPathFromFilename("S:\\a\\b\\c\\"));
180+
ASSERT_EQUALS("S:/tmp/", Path::getPathFromFilename("S:/tmp/index.h"));
181+
ASSERT_EQUALS("S:/a/b/c/", Path::getPathFromFilename("S:/a/b/c/index.h"));
144182
}
145183

146184
void join() const {
147185
ASSERT_EQUALS("a", Path::join("a", ""));
148186
ASSERT_EQUALS("a", Path::join("", "a"));
149187
ASSERT_EQUALS("a/b", Path::join("a", "b"));
150188
ASSERT_EQUALS("a/b", Path::join("a/", "b"));
189+
ASSERT_EQUALS("a/b", Path::join("a\\", "b"));
151190
ASSERT_EQUALS("/b", Path::join("a", "/b"));
191+
ASSERT_EQUALS("/b", Path::join("a", "\\b"));
192+
}
193+
194+
// TODO: this is quite messy - should Path::getAbsoluteFilePath() return normalized separators?
195+
void getAbsoluteFilePath() const {
196+
// Path::getAbsoluteFilePath() only works with existing paths on Linux
197+
#ifdef _WIN32
198+
const std::string cwd = Path::getCurrentPath();
199+
ASSERT_EQUALS(Path::join(cwd, "a.h"), Path::fromNativeSeparators(Path::getAbsoluteFilePath("a.h")));
200+
ASSERT_EQUALS(Path::join(cwd, "inc/a.h"), Path::fromNativeSeparators(Path::getAbsoluteFilePath("inc/a.h")));
201+
const std::string cwd_down = Path::getPathFromFilename(cwd);
202+
ASSERT_EQUALS(Path::join(cwd_down, "a.h"), Path::fromNativeSeparators(Path::getAbsoluteFilePath("../a.h")));
203+
ASSERT_EQUALS(Path::join(cwd_down, "inc/a.h"), Path::fromNativeSeparators(Path::getAbsoluteFilePath("../inc/a.h")));
204+
ASSERT_EQUALS(Path::join(cwd_down, "inc/a.h"), Path::fromNativeSeparators(Path::getAbsoluteFilePath("../inc/../inc/a.h")));
205+
#endif
206+
}
207+
208+
void stripDirectoryPart() const {
209+
ASSERT_EQUALS("a.h", Path::stripDirectoryPart("a.h"));
210+
ASSERT_EQUALS("a.h", Path::stripDirectoryPart("a/a.h"));
211+
ASSERT_EQUALS("a.h", Path::stripDirectoryPart("a/b/a.h"));
212+
ASSERT_EQUALS("a.h", Path::stripDirectoryPart("/mnt/a/b/a.h"));
213+
ASSERT_EQUALS("a.h", Path::stripDirectoryPart("a\\a.h"));
214+
ASSERT_EQUALS("a.h", Path::stripDirectoryPart("a\\b\\a.h"));
215+
ASSERT_EQUALS("a.h", Path::stripDirectoryPart("S:\\a\\b\\a.h"));
216+
ASSERT_EQUALS("a.h", Path::stripDirectoryPart("S:/a/b/a.h"));
217+
152218
}
153219
};
154220

0 commit comments

Comments
 (0)