From b3ef0fc8daa41e433f1919a4933a1cc047f59341 Mon Sep 17 00:00:00 2001 From: Adrian Kummerlaender Date: Sat, 20 Feb 2016 22:30:11 +0100 Subject: Interpose `open` library function `open` is not as side effect free as I had imagined - i.e. if the flag `O_TRUNC` is passed it truncates the file contents alongside opening the file descriptor. In practice this is done by _emacs_ prior to writing the new file content and as such needs to be intercepted so we can start tracking the file before it is changed. Interposing `open` required some changes to make the library work without including `fcntl.h`. This header not only defines some of the flags we require to check if a library call actually is able to change files but also defines the `open` library function. While implementing this change I noticed that the function interpositions implemented in C++ actually need to be declared as `external "C"` so their names do not get wrangled during compilation. I suspect that this was previously implicitly done for e.g. `mmap` and `write` by the included C standard library headers. However this did not work for `open` which is why all function interpositions are now explicitly declared external. End result: _emacs_ file changes are now tracked correctly. --- README.md | 2 +- src/actual.h | 3 --- src/bootstrap.cc | 4 ++++ src/main.cc | 41 ++++++++++++++++++++++++++++++++++++++++- src/tracking/change_tracker.cc | 2 +- src/utility/logger.h | 3 ++- 6 files changed, 48 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index e265747..4d155de 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ The goal is to develop `change` into a utility that can be dropped in front of a ## Filtering -Due to it's nature of interposing low level system calls such as `write` and `unlink` the library by default exposes lots of the internal write logic of the wrapped application. For instance it reports _vim_ creating a file called `4913` to verify the target directory's writability as well as the creation of various temporary backup files. While this is certainly interesting for debugging purposes it hinders the library's goal of providing a higher level summary consisting primarily of the actions the user explicity performed such as the changed file contents. +Due to it's nature of interposing low level calls such as `write` and `unlink` the library by default exposes lots of the internal write logic of the wrapped application. For instance it reports _vim_ creating a file called `4913` to verify the target directory's writability as well as the creation of various temporary backup files. While this is certainly interesting for debugging purposes it hinders the library's goal of providing a higher level summary consisting primarily of the actions the user explicity performed such as the changed file contents. To solve this problem one may provide a list of regular expressions to be matched against the file paths via the `CHANGE_LOG_IGNORE_PATTERN_PATH` environment variable. diff --git a/src/actual.h b/src/actual.h index a860590..ae951b9 100644 --- a/src/actual.h +++ b/src/actual.h @@ -6,9 +6,6 @@ #endif #include -#include -#include - #include #include diff --git a/src/bootstrap.cc b/src/bootstrap.cc index 309ff20..1d509d8 100644 --- a/src/bootstrap.cc +++ b/src/bootstrap.cc @@ -2,6 +2,8 @@ #include "init/alloc.h" +extern "C" { + void free(void* ptr) { static actual::ptr actual_free{}; @@ -41,3 +43,5 @@ void* calloc(size_t block, size_t size) { return actual_calloc(block, size); } } + +} diff --git a/src/main.cc b/src/main.cc index 8b6ca23..7e849fd 100644 --- a/src/main.cc +++ b/src/main.cc @@ -1,3 +1,7 @@ +#include +#include +#include + #include "actual.h" #include "utility/io.h" @@ -61,6 +65,14 @@ inline void track_write(const int fd) { } } +inline void track_write(const std::string& path) { + if ( enabled && utility::is_regular_file(path.c_str()) ) { + if ( !matcher->isMatching(path) ) { + tracker->track(path); + } + } +} + inline void track_rename( const std::string& old_path, const std::string& new_path) { if ( enabled ) { @@ -82,6 +94,28 @@ inline void track_remove(const std::string& path) { } } +extern "C" { + +int open(const char* path, int flags, mode_t mode) { + static actual::ptr actual_open{}; + + if ( !actual_open ) { + actual_open = actual::get_ptr("open"); + } + + // `open` may reset the file contents when used with the `O_TRUNC` flag. + // e.g. this is how _emacs_ clears the file content prior to writing the + // new content. + // Normally `O_TRUNC` is defined in `fcntl.h` which we can not include + // as it defines the very c-function we are currently _overriding_. + // + if ( flags & 01000 ) { + track_write(path); + } + + return actual_open(path, flags, mode); +} + ssize_t write(int fd, const void* buffer, size_t count) { static actual::ptr actual_write{}; @@ -163,7 +197,10 @@ int unlinkat(int dirfd, const char* path, int flags) { actual_unlinkat = actual::get_ptr("unlinkat"); } - if ( dirfd == AT_FDCWD ) { + // Normally `AT_FDCWD` is defined in `fcntl.h` which we can not include + // as it defines `open` which this library also aims to override. + // + if ( dirfd == -100 ) { track_remove(path); } else { track_remove(utility::get_file_path(dirfd) + path); @@ -171,3 +208,5 @@ int unlinkat(int dirfd, const char* path, int flags) { return actual_unlinkat(dirfd, path, flags); } + +} diff --git a/src/tracking/change_tracker.cc b/src/tracking/change_tracker.cc index bc8465e..c966c04 100644 --- a/src/tracking/change_tracker.cc +++ b/src/tracking/change_tracker.cc @@ -1,7 +1,7 @@ #include "change_tracker.h" +#include #include -#include namespace { diff --git a/src/utility/logger.h b/src/utility/logger.h index 0dd88a6..229032c 100644 --- a/src/utility/logger.h +++ b/src/utility/logger.h @@ -5,7 +5,8 @@ #include -#include +#include +#include namespace utility { -- cgit v1.2.3