Skip to content

Commit

Permalink
Move file enumeration to filepaths.
Browse files Browse the repository at this point in the history
Review URL: http://codereview.chromium.org/13315

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@6784 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
avi@google.com committed Dec 11, 2008
1 parent 64596e5 commit 0b73322
Show file tree
Hide file tree
Showing 18 changed files with 126 additions and 100 deletions.
17 changes: 12 additions & 5 deletions base/file_path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ bool FilePath::IsSeparator(CharType character) {
// adhere to their behavior.
FilePath FilePath::DirName() const {
FilePath new_path(path_);
new_path.StripTrailingSeparators();
new_path.StripTrailingSeparatorsInternal();

// The drive letter, if any, always needs to remain in the output. If there
// is no drive letter, as will always be the case on platforms which do not
Expand All @@ -104,7 +104,7 @@ FilePath FilePath::DirName() const {
new_path.path_.resize(last_separator);
}

new_path.StripTrailingSeparators();
new_path.StripTrailingSeparatorsInternal();
if (!new_path.path_.length())
new_path.path_ = kCurrentDirectory;

Expand All @@ -113,7 +113,7 @@ FilePath FilePath::DirName() const {

FilePath FilePath::BaseName() const {
FilePath new_path(path_);
new_path.StripTrailingSeparators();
new_path.StripTrailingSeparatorsInternal();

// The drive letter, if any, is always stripped.
StringType::size_type letter = FindDriveLetter(new_path.path_);
Expand Down Expand Up @@ -148,7 +148,7 @@ FilePath FilePath::Append(const FilePath::StringType& component) const {
}

FilePath new_path(path_);
new_path.StripTrailingSeparators();
new_path.StripTrailingSeparatorsInternal();

// Don't append a separator if the path is empty (indicating the current
// directory) or if the path component is empty (indicating nothing to
Expand Down Expand Up @@ -201,7 +201,14 @@ std::wstring FilePath::ToWStringHack() const {
}
#endif

void FilePath::StripTrailingSeparators() {
FilePath FilePath::StripTrailingSeparators() const {
FilePath new_path(path_);
new_path.StripTrailingSeparatorsInternal();

return new_path;
}

void FilePath::StripTrailingSeparatorsInternal() {
// If there is no drive letter, start will be 1, which will prevent stripping
// the leading separator if there is only one separator. If there is a drive
// letter, start will be set appropriately to prevent stripping the first
Expand Down
13 changes: 11 additions & 2 deletions base/file_path.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@

#include <string>

#include "base/compiler_specific.h"
#include "base/basictypes.h"
#include "base/compiler_specific.h"

// Windows-style drive letter support and pathname separator characters can be
// enabled and disabled independently, to aid testing. These #defines are
Expand Down Expand Up @@ -121,6 +121,11 @@ class FilePath {
return path_ == that.path_;
}

// Required for some STL containers and operations
bool operator<(const FilePath& that) const {
return path_ < that.path_;
}

const StringType& value() const { return path_; }

// Returns true if |character| is in kSeparators.
Expand Down Expand Up @@ -154,6 +159,10 @@ class FilePath {
// platforms, an absolute path begins with a separator character.
bool IsAbsolute() const;

// Returns a copy of this FilePath that does not end with a trailing
// separator.
FilePath StripTrailingSeparators() const;

// Older Chromium code assumes that paths are always wstrings.
// This function converts a wstring to a FilePath, and is useful to smooth
// porting that old code to the FilePath API.
Expand All @@ -174,7 +183,7 @@ class FilePath {
// directory, so "////" will become "/", not "". A leading pair of
// separators is never stripped, to support alternate roots. This is used to
// support UNC paths on Windows.
void StripTrailingSeparators();
void StripTrailingSeparatorsInternal();

StringType path_;
};
Expand Down
16 changes: 9 additions & 7 deletions base/file_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ bool EndsWithSeparator(const std::wstring& path);
bool EnsureEndsWithSeparator(FilePath* path);

// Modifies a string by trimming all trailing separators from the end.
// Deprecated. FilePath does this automatically, and if it's constructed from a
// path with a trailing separator, StripTrailingSeparators() may be used.
void TrimTrailingSeparator(std::wstring* dir);

// Strips the topmost directory from the end of 'dir'. Assumes 'dir' does not
Expand Down Expand Up @@ -373,34 +375,34 @@ class FileEnumerator {
// NOTE: the pattern only matches the contents of root_path, not files in
// recursive subdirectories.
// TODO(erikkay): Fix the pattern matching to work at all levels.
FileEnumerator(const std::wstring& root_path,
FileEnumerator(const FilePath& root_path,
bool recursive,
FileEnumerator::FILE_TYPE file_type);
FileEnumerator(const std::wstring& root_path,
FileEnumerator(const FilePath& root_path,
bool recursive,
FileEnumerator::FILE_TYPE file_type,
const std::wstring& pattern);
const FilePath::StringType& pattern);
~FileEnumerator();

// Returns an empty string if there are no more results.
std::wstring Next();
FilePath Next();

// Write the file info into |info|.
void GetFindInfo(FindInfo* info);

private:
std::wstring root_path_;
FilePath root_path_;
bool recursive_;
FILE_TYPE file_type_;
std::wstring pattern_; // Empty when we want to find everything.
FilePath pattern_; // Empty when we want to find everything.

// Set to true when there is a find operation open. This way, we can lazily
// start the operations when the caller calls Next().
bool is_in_find_op_;

// A stack that keeps track of which subdirectories we still need to
// enumerate in the breadth-first search.
std::stack<std::wstring> pending_paths_;
std::stack<FilePath> pending_paths_;

#if defined(OS_WIN)
WIN32_FIND_DATA find_data_;
Expand Down
24 changes: 12 additions & 12 deletions base/file_util_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ bool SetCurrentDirectory(const FilePath& path) {
return !ret;
}

FileEnumerator::FileEnumerator(const std::wstring& root_path,
FileEnumerator::FileEnumerator(const FilePath& root_path,
bool recursive,
FileEnumerator::FILE_TYPE file_type)
: recursive_(recursive),
Expand All @@ -388,19 +388,19 @@ FileEnumerator::FileEnumerator(const std::wstring& root_path,
pending_paths_.push(root_path);
}

FileEnumerator::FileEnumerator(const std::wstring& root_path,
FileEnumerator::FileEnumerator(const FilePath& root_path,
bool recursive,
FileEnumerator::FILE_TYPE file_type,
const std::wstring& pattern)
const FilePath::StringType& pattern)
: recursive_(recursive),
file_type_(file_type),
pattern_(root_path),
pattern_(root_path.value()),
is_in_find_op_(false),
fts_(NULL) {
// The Windows version of this code only matches against items in the top-most
// directory, and we're comparing fnmatch against full paths, so this is the
// easiest way to get the right pattern.
AppendToPath(&pattern_, pattern);
pattern_ = pattern_.Append(pattern);
pending_paths_.push(root_path);
}

Expand All @@ -423,20 +423,20 @@ void FileEnumerator::GetFindInfo(FindInfo* info) {
// the fts enumeration doesn't match (type, pattern, etc.). In the case of
// large directories with many files this can be quite deep.
// TODO(erikkay) - get rid of this recursive pattern
std::wstring FileEnumerator::Next() {
FilePath FileEnumerator::Next() {
if (!is_in_find_op_) {
if (pending_paths_.empty())
return std::wstring();
return FilePath();

// The last find FindFirstFile operation is done, prepare a new one.
root_path_ = pending_paths_.top();
TrimTrailingSeparator(&root_path_);
root_path_ = root_path_.StripTrailingSeparators();
pending_paths_.pop();

// Start a new find operation.
int ftsflags = FTS_LOGICAL;
char top_dir[PATH_MAX];
base::strlcpy(top_dir, WideToUTF8(root_path_).c_str(), sizeof(top_dir));
base::strlcpy(top_dir, root_path_.value().c_str(), sizeof(top_dir));
char* dir_list[2] = { top_dir, NULL };
fts_ = fts_open(dir_list, ftsflags, NULL);
if (!fts_)
Expand All @@ -458,15 +458,15 @@ std::wstring FileEnumerator::Next() {

// Patterns are only matched on the items in the top-most directory.
// (see Windows implementation)
if (fts_ent_->fts_level == 1 && pattern_.length() > 0) {
if (fnmatch(WideToUTF8(pattern_).c_str(), fts_ent_->fts_path, 0) != 0) {
if (fts_ent_->fts_level == 1 && pattern_.value().length() > 0) {
if (fnmatch(pattern_.value().c_str(), fts_ent_->fts_path, 0) != 0) {
if (fts_ent_->fts_info == FTS_D)
fts_set(fts_, fts_ent_, FTS_SKIP);
return Next();
}
}

std::wstring cur_file(UTF8ToWide(fts_ent_->fts_path));
FilePath cur_file(fts_ent_->fts_path);
if (fts_ent_->fts_info == FTS_D) {
// If not recursive, then prune children.
if (!recursive_)
Expand Down
33 changes: 17 additions & 16 deletions base/file_util_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ class FileUtilTest : public PlatformTest {
class FindResultCollector {
public:
FindResultCollector(file_util::FileEnumerator& enumerator) {
std::wstring cur_file;
while (!(cur_file = enumerator.Next()).empty()) {
FilePath::StringType path = FilePath::FromWStringHack(cur_file).value();
FilePath cur_file;
while (!(cur_file = enumerator.Next()).value().empty()) {
FilePath::StringType path = cur_file.value();
// The file should not be returned twice.
EXPECT_TRUE(files_.end() == files_.find(path))
<< "Same file returned twice";
Expand Down Expand Up @@ -830,10 +830,10 @@ TEST_F(FileUtilTest, ReplaceExtensionTestWithPathSeparators) {

TEST_F(FileUtilTest, FileEnumeratorTest) {
// Test an empty directory.
file_util::FileEnumerator f0(test_dir_.ToWStringHack(), true,
file_util::FileEnumerator f0(test_dir_, true,
file_util::FileEnumerator::FILES_AND_DIRECTORIES);
EXPECT_EQ(f0.Next(), L"");
EXPECT_EQ(f0.Next(), L"");
EXPECT_EQ(f0.Next().value(), FILE_PATH_LITERAL(""));
EXPECT_EQ(f0.Next().value(), FILE_PATH_LITERAL(""));

// create the directories
FilePath dir1 = test_dir_.Append(FILE_PATH_LITERAL("dir1"));
Expand All @@ -857,7 +857,7 @@ TEST_F(FileUtilTest, FileEnumeratorTest) {
FilePath file2_abs = test_dir_.Append(FILE_PATH_LITERAL("file2.txt"));

// Only enumerate files.
file_util::FileEnumerator f1(test_dir_.ToWStringHack(), true,
file_util::FileEnumerator f1(test_dir_, true,
file_util::FileEnumerator::FILES);
FindResultCollector c1(f1);
EXPECT_TRUE(c1.HasFile(file1));
Expand All @@ -867,7 +867,7 @@ TEST_F(FileUtilTest, FileEnumeratorTest) {
EXPECT_EQ(c1.size(), 4);

// Only enumerate directories.
file_util::FileEnumerator f2(test_dir_.ToWStringHack(), true,
file_util::FileEnumerator f2(test_dir_, true,
file_util::FileEnumerator::DIRECTORIES);
FindResultCollector c2(f2);
EXPECT_TRUE(c2.HasFile(dir1));
Expand All @@ -877,14 +877,14 @@ TEST_F(FileUtilTest, FileEnumeratorTest) {

// Only enumerate directories non-recursively.
file_util::FileEnumerator f2_non_recursive(
test_dir_.ToWStringHack(), false, file_util::FileEnumerator::DIRECTORIES);
test_dir_, false, file_util::FileEnumerator::DIRECTORIES);
FindResultCollector c2_non_recursive(f2_non_recursive);
EXPECT_TRUE(c2_non_recursive.HasFile(dir1));
EXPECT_TRUE(c2_non_recursive.HasFile(dir2));
EXPECT_EQ(c2_non_recursive.size(), 2);

// Enumerate files and directories.
file_util::FileEnumerator f3(test_dir_.ToWStringHack(), true,
file_util::FileEnumerator f3(test_dir_, true,
file_util::FileEnumerator::FILES_AND_DIRECTORIES);
FindResultCollector c3(f3);
EXPECT_TRUE(c3.HasFile(dir1));
Expand All @@ -897,7 +897,7 @@ TEST_F(FileUtilTest, FileEnumeratorTest) {
EXPECT_EQ(c3.size(), 7);

// Non-recursive operation.
file_util::FileEnumerator f4(test_dir_.ToWStringHack(), false,
file_util::FileEnumerator f4(test_dir_, false,
file_util::FileEnumerator::FILES_AND_DIRECTORIES);
FindResultCollector c4(f4);
EXPECT_TRUE(c4.HasFile(dir2));
Expand All @@ -907,8 +907,9 @@ TEST_F(FileUtilTest, FileEnumeratorTest) {
EXPECT_EQ(c4.size(), 4);

// Enumerate with a pattern.
file_util::FileEnumerator f5(test_dir_.ToWStringHack(), true,
file_util::FileEnumerator::FILES_AND_DIRECTORIES, L"dir*");
file_util::FileEnumerator f5(test_dir_, true,
file_util::FileEnumerator::FILES_AND_DIRECTORIES,
FILE_PATH_LITERAL("dir*"));
FindResultCollector c5(f5);
EXPECT_TRUE(c5.HasFile(dir1));
EXPECT_TRUE(c5.HasFile(dir2));
Expand All @@ -919,10 +920,10 @@ TEST_F(FileUtilTest, FileEnumeratorTest) {

// Make sure the destructor closes the find handle while in the middle of a
// query to allow TearDown to delete the directory.
file_util::FileEnumerator f6(test_dir_.ToWStringHack(), true,
file_util::FileEnumerator f6(test_dir_, true,
file_util::FileEnumerator::FILES_AND_DIRECTORIES);
EXPECT_FALSE(f6.Next().empty()); // Should have found something
// (we don't care what).
EXPECT_FALSE(f6.Next().value().empty()); // Should have found something
// (we don't care what).
}


Expand Down
Loading

0 comments on commit 0b73322

Please sign in to comment.