From f8eecef184c8684a3ed27712ccf8f7f866d47c40 Mon Sep 17 00:00:00 2001 From: Adrian Kummerlaender Date: Sun, 14 Feb 2016 13:31:59 +0100 Subject: Reduce `ChangeTracker` public _interface_ to `track` --- src/change_log.cc | 12 ++---- src/tracking/change_tracker.cc | 93 ++++++++++++++++++++++-------------------- src/tracking/change_tracker.h | 9 ++-- src/utility/io.h | 2 +- 4 files changed, 57 insertions(+), 59 deletions(-) diff --git a/src/change_log.cc b/src/change_log.cc index ae28079..a00a26f 100644 --- a/src/change_log.cc +++ b/src/change_log.cc @@ -42,20 +42,14 @@ void init() { inline void track_write(const int fd) { if ( enabled && fd != *fd_guard && utility::is_regular_file(fd) ) { - const std::string file_name{ utility::get_file_name(fd) }; - - if ( !tracker->is_tracked(file_name) ) { - tracker->track(file_name); - } + tracker->track(utility::get_file_path(fd)); } } inline void track_rename( const std::string& old_path, const std::string& new_path) { if ( enabled ) { - if ( !tracker->is_tracked(old_path) ) { - tracker->track(old_path); - } + tracker->track(old_path); logger->append("renamed '", old_path, "' to '", new_path, "'"); } @@ -109,7 +103,7 @@ int unlinkat(int dirfd, const char* path, int flags) { if ( dirfd == AT_FDCWD ) { track_remove(path); } else { - track_remove(utility::get_file_name(dirfd) + path); + track_remove(utility::get_file_path(dirfd) + path); } return actual::unlinkat(dirfd, path, flags); diff --git a/src/tracking/change_tracker.cc b/src/tracking/change_tracker.cc index 0feae34..df91be0 100644 --- a/src/tracking/change_tracker.cc +++ b/src/tracking/change_tracker.cc @@ -34,6 +34,25 @@ std::string getDiffCommand( return diff_cmd + " --label " + file_path + " - " + file_path; } +void read_file_to_stream( + const boost::filesystem::path& file_path, + std::stringstream* const stream +) { + try { + boost::filesystem::ifstream file(file_path); + + if ( file.is_open() ) { + *stream << file.rdbuf(); + stream->sync(); + } + } catch ( boost::filesystem::filesystem_error& ) { + // we catch this exception in case the parent process is + // performing system calls that are allowed to fail - e.g. + // in this instance writing to files that we are not allowed + // to read. + } +} + } namespace tracking { @@ -68,64 +87,48 @@ ChangeTracker::~ChangeTracker() { } } -// Both `is_tracked` and `create_child` may throw filesystem exceptions -// if the given path doesn't exist. We are able to disregard this as -// both functions are only and should only be called when existance is -// guaranteed. If this is not the case we want the library to fail visibly. - -bool ChangeTracker::is_tracked(const std::string& file_path) const { - return this->children_.find( - boost::filesystem::canonical(file_path).string() - ) != this->children_.end(); +bool ChangeTracker::is_tracked(const boost::filesystem::path& file_path) const { + return this->children_.find(file_path.string()) != this->children_.end(); } -auto ChangeTracker::create_child(const std::string& file_path) { +auto ChangeTracker::create_child(const boost::filesystem::path& file_path) { std::lock_guard guard(this->write_mutex_); return this->children_.emplace( - boost::filesystem::canonical(file_path).string(), + file_path.string(), std::make_unique() ); } -// Begins tracking changes to a file reachable by a given path +// _Tracking_ consists of adding the full file path to the `children_` map and +// reading the full pre-change file contents into the mapped `std::stringstream` +// instance. // -// _Tracking_ consists of adding the full file path to the `children_` -// map and reading the full pre-change file contents into the mapped -// `std::stringstream` instance. +// This seems to be the most workable of various tested approaches to providing +// `diff` with proper input in all circumstances. Leaving the intermediate +// preservation of the original file content to `diff` sadly failed randomly as +// it is _too intelligent_ in actually reading the file from permanent storage. +// e.g. it opens all relevant file descriptors at launch and only reads the +// first block of the file contents until more is required. This leads to problems +// when the tracked file is modified after `diff` has been spawned. // -// This seems to be the most workable of various tested approaches -// to providing `diff` with proper input in all circumstances. Leaving -// the intermediate preservation of the original file content to -// `diff` sadly failed randomly as it is _too intelligent_ in actually -// reading the file from permanent storage. e.g. it opens all relevant -// file descriptors at launch and only reads the first block of the -// file contents until more is required. This leads to problems when -// the tracked file is modified after `diff` has been spawned. -// -bool ChangeTracker::track(const std::string& file_path) { - auto result = this->create_child(file_path); - - if ( std::get(result) ) { - try { - boost::filesystem::ifstream file( - std::get(*result.first) +void ChangeTracker::track(const std::string& file_path) { + // May throw filesystem exceptions if the given path doesn't exist. We are + // able to disregard this as this function should only be called when + // existance is guaranteed. If this is not the case we want the library to + // fail visibly. + // + const auto full_path = boost::filesystem::canonical(file_path); + + if ( !this->is_tracked(full_path) ) { + auto result = this->create_child(full_path); + + if ( std::get(result) ) { + read_file_to_stream( + full_path, + std::get(*result.first).get() ); - - if ( file.is_open() ) { - *std::get(*result.first) << file.rdbuf(); - std::get(*result.first)->sync(); - } - } catch ( boost::filesystem::filesystem_error& ) { - // we catch this exception in case the parent process is - // performing system calls that are allowed to fail - e.g. - // in this instance writing to files that we are not allowed - // to read. } - - return true; - } else { - return false; } } diff --git a/src/tracking/change_tracker.h b/src/tracking/change_tracker.h index 04a5a00..a9c9800 100644 --- a/src/tracking/change_tracker.h +++ b/src/tracking/change_tracker.h @@ -15,9 +15,8 @@ class ChangeTracker { ChangeTracker(utility::Logger*); ~ChangeTracker(); - bool is_tracked(const std::string&) const; - - bool track(const std::string&); + // Begin tracking changes to a given path if not already covered. + void track(const std::string&); private: const std::string diff_cmd_; @@ -29,8 +28,10 @@ class ChangeTracker { std::unique_ptr > children_; + bool is_tracked(const boost::filesystem::path&) const; + // threadsafe child emplacement - auto create_child(const std::string&); + auto create_child(const boost::filesystem::path&); }; diff --git a/src/utility/io.h b/src/utility/io.h index 32d6ecb..4c09b30 100644 --- a/src/utility/io.h +++ b/src/utility/io.h @@ -33,7 +33,7 @@ class FileDescriptorGuard { }; -std::string get_file_name(int fd) { +std::string get_file_path(int fd) { char proc_link[20]; char file_name[256]; -- cgit v1.2.3