From 70d62c4f937b8f7097affa45d028c462ae704a5e Mon Sep 17 00:00:00 2001 From: firewave Date: Mon, 22 Apr 2024 11:22:41 +0200 Subject: [PATCH 1/3] test/cli/other_test.py: added test for `FileLister` --- test/cli/other_test.py | 72 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-) diff --git a/test/cli/other_test.py b/test/cli/other_test.py index 1ac8e4a341e..ec3d15c7c68 100644 --- a/test/cli/other_test.py +++ b/test/cli/other_test.py @@ -1357,4 +1357,74 @@ def test_rule(tmpdir): lines = stderr.splitlines() assert lines == [ "{}:4:0: style: found 'f' [rule]".format(test_file) - ] \ No newline at end of file + ] + + +def test_filelist(tmpdir): + list_dir = os.path.join(tmpdir, 'list-dir') + os.mkdir(list_dir) + + with open(os.path.join(list_dir, 'aaa.c'), 'wt'): + pass + with open(os.path.join(list_dir, 'zzz.c'), 'wt'): + pass + with open(os.path.join(list_dir, 'valueflow.cpp'), 'wt'): + pass + with open(os.path.join(list_dir, 'vfvalue.cpp'), 'wt'): + pass + with open(os.path.join(list_dir, 'vf_enumvalue.cpp'), 'wt'): + pass + with open(os.path.join(list_dir, 'vf_analyze.h'), 'wt'): + pass + + sub_dir_1 = os.path.join(list_dir, 'valueflow') + os.mkdir(sub_dir_1) + with open(os.path.join(sub_dir_1, 'file.cpp'), 'wt'): + pass + with open(os.path.join(sub_dir_1, 'file.c'), 'wt'): + pass + with open(os.path.join(sub_dir_1, 'file.h'), 'wt'): + pass + + sub_dir_2 = os.path.join(list_dir, 'vfvalue') + os.mkdir(sub_dir_2) + with open(os.path.join(sub_dir_2, 'file.cpp'), 'wt'): + pass + with open(os.path.join(sub_dir_2, 'file.c'), 'wt'): + pass + with open(os.path.join(sub_dir_2, 'file.h'), 'wt'): + pass + + sub_dir_3 = os.path.join(list_dir, 'vf_enumvalue') + os.mkdir(sub_dir_3) + with open(os.path.join(sub_dir_3, 'file.cpp'), 'wt'): + pass + with open(os.path.join(sub_dir_3, 'file.c'), 'wt'): + pass + with open(os.path.join(sub_dir_3, 'file.h'), 'wt'): + pass + + # TODO: -rp is not applied to "Checking" messages + #exitcode, stdout, _ = cppcheck(['-j1', '-rp', list_dir]) + exitcode, stdout, _ = cppcheck(['-j1', '.'], cwd=list_dir) + assert exitcode == 0, stdout + if sys.platform == "win32": + stdout = stdout.replace('\\', '/') + lines = stdout.splitlines() + expected = [ + 'Checking aaa.c ...', + 'Checking valueflow.cpp ...', + 'Checking valueflow/file.c ...', + 'Checking valueflow/file.cpp ...', + 'Checking vf_enumvalue.cpp ...', + 'Checking vf_enumvalue/file.c ...', + 'Checking vf_enumvalue/file.cpp ...', + 'Checking vfvalue.cpp ...', + 'Checking vfvalue/file.c ...', + 'Checking vfvalue/file.cpp ...', + 'Checking zzz.c ...' + ] + assert len(expected), len(lines) + for i in range(1, len(expected)+1): + lines.remove('{}/11 files checked 0% done'.format(i, len(expected))) + assert lines == expected From 044eaf3afa4fc41c4d68d2ef5d3fc9bfdcb40a25 Mon Sep 17 00:00:00 2001 From: firewave Date: Mon, 22 Apr 2024 10:51:57 +0200 Subject: [PATCH 2/3] FileLister: small cleanup --- cli/filelister.cpp | 130 ++++++++++++++++++++++++--------------------- 1 file changed, 68 insertions(+), 62 deletions(-) diff --git a/cli/filelister.cpp b/cli/filelister.cpp index 81f8e51020e..7e5fcfe679b 100644 --- a/cli/filelister.cpp +++ b/cli/filelister.cpp @@ -41,16 +41,8 @@ // When compiling Unicode targets WinAPI automatically uses *W Unicode versions // of called functions. Thus, we explicitly call *A versions of the functions. -std::string FileLister::recursiveAddFiles(std::list&files, const std::string &path, const std::set &extra, const PathMatch& ignored) +static std::string addFiles2(std::list&files, const std::string &path, const std::set &extra, bool recursive, const PathMatch& ignored) { - return addFiles(files, path, extra, true, ignored); -} - -std::string FileLister::addFiles(std::list&files, const std::string &path, const std::set &extra, bool recursive, const PathMatch& ignored) -{ - if (path.empty()) - return "no path specified"; - const std::string cleanedPath = Path::toNativeSeparators(path); // basedir is the base directory which is used to form pathnames. @@ -110,14 +102,16 @@ std::string FileLister::addFiles(std::list&files, const std::st if ((ffd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) == 0) { // File if ((!checkAllFilesInDir || Path::acceptFile(fname, extra)) && !ignored.match(fname)) { - const std::string nativename = Path::fromNativeSeparators(fname); + std::string nativename = Path::fromNativeSeparators(fname); // Limitation: file sizes are assumed to fit in a 'size_t' #ifdef _WIN64 - files.emplace_back(nativename, (static_cast(ffd.nFileSizeHigh) << 32) | ffd.nFileSizeLow); + const std::size_t filesize = (static_cast(ffd.nFileSizeHigh) << 32) | ffd.nFileSizeLow; #else - files.emplace_back(nativename, ffd.nFileSizeLow); + const std::size_t filesize = ffd.nFileSizeLow; + #endif + files.emplace_back(std::move(nativename), filesize); } } else { // Directory @@ -125,7 +119,7 @@ std::string FileLister::addFiles(std::list&files, const std::st if (!ignored.match(fname)) { std::list filesSorted; - std::string err = FileLister::recursiveAddFiles(filesSorted, fname, extra, ignored); + std::string err = addFiles2(filesSorted, fname, extra, recursive, ignored); if (!err.empty()) return err; @@ -152,6 +146,14 @@ std::string FileLister::addFiles(std::list&files, const std::st return ""; } +std::string FileLister::addFiles(std::list &files, const std::string &path, const std::set &extra, bool recursive, const PathMatch& ignored) +{ + if (path.empty()) + return "no path specified"; + + return addFiles2(files, path, extra, recursive, ignored); +} + #else /////////////////////////////////////////////////////////////////////////////// @@ -177,68 +179,67 @@ static std::string addFiles2(std::list &files, return ""; struct stat file_stat; - if (stat(path.c_str(), &file_stat) != -1) { - if ((file_stat.st_mode & S_IFMT) == S_IFDIR) { - DIR * dir = opendir(path.c_str()); - if (!dir) { - const int err = errno; - return "could not open directory '" + path + "' (errno: " + std::to_string(err) + ")"; - } - std::unique_ptr dir_deleter(dir, closedir); + if (stat(path.c_str(), &file_stat) == -1) + return ""; // TODO: return error? + if ((file_stat.st_mode & S_IFMT) != S_IFDIR) + { + files.emplace_back(path, file_stat.st_size); + return ""; + } + + // process directory entry + + DIR * dir = opendir(path.c_str()); + if (!dir) { + const int err = errno; + return "could not open directory '" + path + "' (errno: " + std::to_string(err) + ")"; + } + std::unique_ptr dir_deleter(dir, closedir); - std::string new_path = path; - new_path += '/'; + std::string new_path = path; + new_path += '/'; - std::list filesSorted; + std::list filesSorted; - while (const dirent* dir_result = readdir(dir)) { - if ((std::strcmp(dir_result->d_name, ".") == 0) || - (std::strcmp(dir_result->d_name, "..") == 0)) - continue; + while (const dirent* dir_result = readdir(dir)) { + if ((std::strcmp(dir_result->d_name, ".") == 0) || + (std::strcmp(dir_result->d_name, "..") == 0)) + continue; - new_path.erase(path.length() + 1); - new_path += dir_result->d_name; + new_path.erase(path.length() + 1); + new_path += dir_result->d_name; #if defined(_DIRENT_HAVE_D_TYPE) || defined(_BSD_SOURCE) - const bool path_is_directory = (dir_result->d_type == DT_DIR || (dir_result->d_type == DT_UNKNOWN && Path::isDirectory(new_path))); + const bool path_is_directory = (dir_result->d_type == DT_DIR || (dir_result->d_type == DT_UNKNOWN && Path::isDirectory(new_path))); #else - const bool path_is_directory = Path::isDirectory(new_path); + const bool path_is_directory = Path::isDirectory(new_path); #endif - if (path_is_directory) { - if (recursive && !ignored.match(new_path)) { - std::string err = addFiles2(filesSorted, new_path, extra, recursive, ignored); - if (!err.empty()) { - return err; - } - } - } else { - if (Path::acceptFile(new_path, extra) && !ignored.match(new_path)) { - if (stat(new_path.c_str(), &file_stat) != -1) { - filesSorted.emplace_back(new_path, file_stat.st_size); - } - else { - const int err = errno; - return "could not stat file '" + new_path + "' (errno: " + std::to_string(err) + ")"; - } - } + if (path_is_directory) { + if (recursive && !ignored.match(new_path)) { + std::string err = addFiles2(filesSorted, new_path, extra, recursive, ignored); + if (!err.empty()) { + return err; + } + } + } else { + if (Path::acceptFile(new_path, extra) && !ignored.match(new_path)) { + if (stat(new_path.c_str(), &file_stat) == -1) { + const int err = errno; + return "could not stat file '" + new_path + "' (errno: " + std::to_string(err) + ")"; } + filesSorted.emplace_back(new_path, file_stat.st_size); } + } + } - // files inside directories need to be sorted as the filesystem doesn't provide a stable order - filesSorted.sort([](const FileWithDetails& a, const FileWithDetails& b) { - return a.path() < b.path(); - }); + // files inside directories need to be sorted as the filesystem doesn't provide a stable order + filesSorted.sort([](const FileWithDetails& a, const FileWithDetails& b) { + return a.path() < b.path(); + }); - files.insert(files.end(), std::make_move_iterator(filesSorted.begin()), std::make_move_iterator(filesSorted.end())); - } else - files.emplace_back(path, file_stat.st_size); - } - return ""; -} + files.insert(files.end(), std::make_move_iterator(filesSorted.begin()), std::make_move_iterator(filesSorted.end())); -std::string FileLister::recursiveAddFiles(std::list &files, const std::string &path, const std::set &extra, const PathMatch& ignored) -{ - return addFiles(files, path, extra, true, ignored); + return ""; } std::string FileLister::addFiles(std::list &files, const std::string &path, const std::set &extra, bool recursive, const PathMatch& ignored) @@ -254,3 +255,8 @@ std::string FileLister::addFiles(std::list &files, const std::s } #endif + +std::string FileLister::recursiveAddFiles(std::list &files, const std::string &path, const std::set &extra, const PathMatch& ignored) +{ + return addFiles(files, path, extra, true, ignored); +} From 84282acad01234b52c4fcc5290284f3dce4b157b Mon Sep 17 00:00:00 2001 From: firewave Date: Mon, 22 Apr 2024 11:26:58 +0200 Subject: [PATCH 3/3] FileLister: ensure stable sorting of files --- cli/filelister.cpp | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/cli/filelister.cpp b/cli/filelister.cpp index 7e5fcfe679b..a960f592fb0 100644 --- a/cli/filelister.cpp +++ b/cli/filelister.cpp @@ -151,7 +151,17 @@ std::string FileLister::addFiles(std::list &files, const std::s if (path.empty()) return "no path specified"; - return addFiles2(files, path, extra, recursive, ignored); + std::list filesSorted; + + std::string err = addFiles2(filesSorted, path, extra, recursive, ignored); + + // files need to be sorted as the filesystems dosn't provide a stable order + filesSorted.sort([](const FileWithDetails& a, const FileWithDetails& b) { + return a.path() < b.path(); + }); + files.insert(files.end(), std::make_move_iterator(filesSorted.begin()), std::make_move_iterator(filesSorted.end())); + + return err; } #else @@ -199,8 +209,6 @@ static std::string addFiles2(std::list &files, std::string new_path = path; new_path += '/'; - std::list filesSorted; - while (const dirent* dir_result = readdir(dir)) { if ((std::strcmp(dir_result->d_name, ".") == 0) || (std::strcmp(dir_result->d_name, "..") == 0)) @@ -216,7 +224,7 @@ static std::string addFiles2(std::list &files, #endif if (path_is_directory) { if (recursive && !ignored.match(new_path)) { - std::string err = addFiles2(filesSorted, new_path, extra, recursive, ignored); + std::string err = addFiles2(files, new_path, extra, recursive, ignored); if (!err.empty()) { return err; } @@ -227,18 +235,11 @@ static std::string addFiles2(std::list &files, const int err = errno; return "could not stat file '" + new_path + "' (errno: " + std::to_string(err) + ")"; } - filesSorted.emplace_back(new_path, file_stat.st_size); + files.emplace_back(new_path, file_stat.st_size); } } } - // files inside directories need to be sorted as the filesystem doesn't provide a stable order - filesSorted.sort([](const FileWithDetails& a, const FileWithDetails& b) { - return a.path() < b.path(); - }); - - files.insert(files.end(), std::make_move_iterator(filesSorted.begin()), std::make_move_iterator(filesSorted.end())); - return ""; } @@ -251,7 +252,17 @@ std::string FileLister::addFiles(std::list &files, const std::s if (endsWith(corrected_path, '/')) corrected_path.erase(corrected_path.end() - 1); - return addFiles2(files, corrected_path, extra, recursive, ignored); + std::list filesSorted; + + std::string err = addFiles2(filesSorted, corrected_path, extra, recursive, ignored); + + // files need to be sorted as the filesystems dosn't provide a stable order + filesSorted.sort([](const FileWithDetails& a, const FileWithDetails& b) { + return a.path() < b.path(); + }); + files.insert(files.end(), std::make_move_iterator(filesSorted.begin()), std::make_move_iterator(filesSorted.end())); + + return err; } #endif