Skip to content

FileLister: ensure sorting of files / some cleanups #6322

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
firewave merged 3 commits into from
Apr 22, 2024
Merged

FileLister: ensure sorting of files / some cleanups #6322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
70d62c4
test/cli/other_test.py: added test for `FileLister`
firewave Apr 22, 2024
044eaf3
FileLister: small cleanup
firewave Apr 22, 2024
84282ac
FileLister: ensure stable sorting of files
firewave Apr 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
145 changes: 81 additions & 64 deletions cli/filelister.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<FileWithDetails>&files, const std::string &path, const std::set<std::string> &extra, const PathMatch& ignored)
static std::string addFiles2(std::list<FileWithDetails>&files, const std::string &path, const std::set<std::string> &extra, bool recursive, const PathMatch& ignored)
{
return addFiles(files, path, extra, true, ignored);
}

std::string FileLister::addFiles(std::list<FileWithDetails>&files, const std::string &path, const std::set<std::string> &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.
Expand Down Expand Up @@ -110,22 +102,24 @@ std::string FileLister::addFiles(std::list<FileWithDetails>&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<std::size_t>(ffd.nFileSizeHigh) << 32) | ffd.nFileSizeLow);
const std::size_t filesize = (static_cast<std::size_t>(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
if (recursive) {
if (!ignored.match(fname)) {
std::list<FileWithDetails> filesSorted;

std::string err = FileLister::recursiveAddFiles(filesSorted, fname, extra, ignored);
std::string err = addFiles2(filesSorted, fname, extra, recursive, ignored);
if (!err.empty())
return err;

Expand All @@ -152,6 +146,24 @@ std::string FileLister::addFiles(std::list<FileWithDetails>&files, const std::st
return "";
}

std::string FileLister::addFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, bool recursive, const PathMatch& ignored)
{
if (path.empty())
return "no path specified";

std::list<FileWithDetails> 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

///////////////////////////////////////////////////////////////////////////////
Expand All @@ -177,68 +189,58 @@ static std::string addFiles2(std::list<FileWithDetails> &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, decltype(&closedir)> 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 "";
}

std::string new_path = path;
new_path += '/';
// 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, decltype(&closedir)> dir_deleter(dir, closedir);

std::list<FileWithDetails> filesSorted;
std::string new_path = path;
new_path += '/';

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(files, new_path, extra, recursive, ignored);
if (!err.empty()) {
return err;
}
}

// 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);
} 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) + ")";
}
files.emplace_back(new_path, file_stat.st_size);
}
}
}
return "";
}

std::string FileLister::recursiveAddFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, const PathMatch& ignored)
{
return addFiles(files, path, extra, true, ignored);
return "";
}

std::string FileLister::addFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, bool recursive, const PathMatch& ignored)
Expand All @@ -250,7 +252,22 @@ std::string FileLister::addFiles(std::list<FileWithDetails> &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<FileWithDetails> 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

std::string FileLister::recursiveAddFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, const PathMatch& ignored)
{
return addFiles(files, path, extra, true, ignored);
}
72 changes: 71 additions & 1 deletion test/cli/other_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1357,4 +1357,74 @@ def test_rule(tmpdir):
lines = stderr.splitlines()
assert lines == [
"{}:4:0: style: found 'f' [rule]".format(test_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