diff --git a/src/FS.cpp b/src/FS.cpp index 3aa9fec..62cd30d 100644 --- a/src/FS.cpp +++ b/src/FS.cpp @@ -366,6 +366,31 @@ std::vector Path::glob (const std::string& pattern) return results; } +//////////////////////////////////////////////////////////////////////////////// +// S_IFMT 0170000 type of file +// S_IFIFO 0010000 named pipe (fifo) +// S_IFCHR 0020000 character special +// S_IFDIR 0040000 directory +// S_IFBLK 0060000 block special +// S_IFREG 0100000 regular +// S_IFLNK 0120000 symbolic link +// S_IFSOCK 0140000 socket +// S_IFWHT 0160000 whiteout +// S_ISUID 0004000 set user id on execution +// S_ISGID 0002000 set group id on execution +// S_ISVTX 0001000 save swapped text even after use +// S_IRUSR 0000400 read permission, owner +// S_IWUSR 0000200 write permission, owner +// S_IXUSR 0000100 execute/search permission, owner +mode_t Path::mode () +{ + struct stat s; + if (stat (_data.c_str (), &s)) + throw format ("stat error {1}: {2}", errno, strerror (errno)); + + return s.st_mode; +} + //////////////////////////////////////////////////////////////////////////////// File::File () : Path::Path () @@ -674,31 +699,6 @@ void File::truncate () throw format ("ftruncate error {1}: {2}", errno, strerror (errno)); } -//////////////////////////////////////////////////////////////////////////////// -// S_IFMT 0170000 type of file -// S_IFIFO 0010000 named pipe (fifo) -// S_IFCHR 0020000 character special -// S_IFDIR 0040000 directory -// S_IFBLK 0060000 block special -// S_IFREG 0100000 regular -// S_IFLNK 0120000 symbolic link -// S_IFSOCK 0140000 socket -// S_IFWHT 0160000 whiteout -// S_ISUID 0004000 set user id on execution -// S_ISGID 0002000 set group id on execution -// S_ISVTX 0001000 save swapped text even after use -// S_IRUSR 0000400 read permission, owner -// S_IWUSR 0000200 write permission, owner -// S_IXUSR 0000100 execute/search permission, owner -mode_t File::mode () -{ - struct stat s; - if (stat (_data.c_str (), &s)) - throw format ("stat error {1}: {2}", errno, strerror (errno)); - - return s.st_mode; -} - //////////////////////////////////////////////////////////////////////////////// size_t File::size () const { @@ -893,6 +893,150 @@ bool File::move (const std::string& from, const std::string& to) return false; } +//////////////////////////////////////////////////////////////////////////////// +AtomicFile::AtomicFile () +: Path::Path () +, _original_file (File ()) +, _new_file (File ()) +, _new_file_in_use (false) +{ +} + +//////////////////////////////////////////////////////////////////////////////// +AtomicFile::AtomicFile (const std::string& in) +: Path::Path (in) +, _original_file (File (in)) +, _new_file (File (in + ".new")) +, _new_file_in_use (false) +{ + assert_no_new_file (); +} + +//////////////////////////////////////////////////////////////////////////////// +AtomicFile& AtomicFile::operator= (const AtomicFile& other) +{ + if (this != &other) { + Path::operator= (other); + this->_original_file = File (other._data); + this->_new_file = File (other._data + ".new"); + + assert_no_new_file (); + } + + return *this; +} + +//////////////////////////////////////////////////////////////////////////////// +void AtomicFile::truncate () +{ + // Instead of truncating original file, we create new file. Subsequent writes + // will go to it. Once all writes are done, we will rename new file on top + // of original one. + if (! _new_file.create()) { + throw format ("Unable to create {1}", _new_file.name()); + } + _new_file_in_use = true; +} + +//////////////////////////////////////////////////////////////////////////////// +size_t AtomicFile::size () const +{ + return _original_file.size (); +} + +//////////////////////////////////////////////////////////////////////////////// +void AtomicFile::close () +{ + if (_new_file_in_use) + { + _new_file.close (); + _original_file.close (); + if (! _new_file.rename (_original_file._data)) { + throw format( + "Unable to rename {1} to {2}", + _new_file.name (), + _original_file.name ()); + } + _new_file_in_use = false; + } + else + { + _original_file.close (); + } +} + +//////////////////////////////////////////////////////////////////////////////// +void AtomicFile::read (std::vector & contents) +{ + if (_new_file_in_use) { + throw "Can't read after overwrite"; + } + _original_file.read (contents); +} + +//////////////////////////////////////////////////////////////////////////////// +bool AtomicFile::lock () +{ + return _original_file.lock (); +} + +//////////////////////////////////////////////////////////////////////////////// +bool AtomicFile::open () +{ + return _original_file.open (); +} + +//////////////////////////////////////////////////////////////////////////////// +void AtomicFile::append (const std::vector & lines) +{ + if (_new_file_in_use) + { + _new_file.append (lines); + } + else + { + _original_file.append (lines); + } +} + +//////////////////////////////////////////////////////////////////////////////// +void AtomicFile::append (const std::string& line) +{ + if (_new_file_in_use) + { + _new_file.append (line); + } + else + { + _original_file.append (line); + } +} + +//////////////////////////////////////////////////////////////////////////////// +void AtomicFile::write_raw (const std::string& line) +{ + if (_new_file_in_use) + { + _new_file.write_raw (line); + } + else + { + _original_file.write_raw (line); + } +} + +//////////////////////////////////////////////////////////////////////////////// +void AtomicFile::assert_no_new_file () +{ + if (_new_file.exists ()) { + throw format ( + "Temporary file {1} already exists. " + "This is likely caused by previous crash. Remove it to continue.", + _new_file.name () + ); + } +} + //////////////////////////////////////////////////////////////////////////////// Directory::Directory () { @@ -900,25 +1044,25 @@ Directory::Directory () //////////////////////////////////////////////////////////////////////////////// Directory::Directory (const Directory& other) -: File::File (other) +: Path::Path (other) { } //////////////////////////////////////////////////////////////////////////////// Directory::Directory (const File& other) -: File::File (other) +: Path::Path (other) { } //////////////////////////////////////////////////////////////////////////////// Directory::Directory (const Path& other) -: File::File (other) +: Path::Path (other) { } //////////////////////////////////////////////////////////////////////////////// Directory::Directory (const std::string& in) -: File::File (in) +: Path::Path (in) { } @@ -926,7 +1070,7 @@ Directory::Directory (const std::string& in) Directory& Directory::operator= (const Directory& other) { if (this != &other) - File::operator= (other); + Path::operator= (other); return *this; } diff --git a/src/FS.h b/src/FS.h index 14222cc..141ea7b 100644 --- a/src/FS.h +++ b/src/FS.h @@ -61,6 +61,8 @@ class Path bool executable () const; bool rename (const std::string&); + mode_t mode (); + // Statics static std::string expand (const std::string&); static std::vector glob (const std::string&); @@ -99,7 +101,6 @@ class File : public Path void truncate (); - virtual mode_t mode (); virtual size_t size () const; virtual time_t mtime () const; virtual time_t ctime () const; @@ -121,7 +122,50 @@ class File : public Path bool _locked; }; -class Directory : public File +// AtomicFile class. +// Implements atomic file rewrite, or at least something close to it - +// implementing fault-tolerant writes is mighty difficult. Main idea is that +// instead of in-place truncate + write we create a completely new file, +// write new version of the data into it, and rename it on top of the previous +// version. +// +// The implementation is heavily based/influenced by AtomicFile.cpp from +// timewarrior: +// https://github.com/GothenburgBitFactory/timewarrior/blob/v1.4.3/src/AtomicFile.cpp +// +// See discussion in +// https://github.com/GothenburgBitFactory/taskwarrior/issues/152 +class AtomicFile : public Path +{ +public: + AtomicFile (); + AtomicFile (const std::string&); + + AtomicFile& operator= (const AtomicFile&); + + bool open (); + void close (); + + bool lock (); + + void read (std::vector &); + void truncate (); + void append (const std::string&); + void append (const std::vector &); + void write_raw (const std::string&); + + size_t size () const; +private: + File _original_file; + File _new_file; + bool _new_file_in_use; + + // Ensures .new file does not exists. + // throws exception if it does. + void assert_no_new_file (); +}; + +class Directory : public Path { public: Directory (); diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 141a3e3..7a60cf2 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -8,6 +8,12 @@ if(POLICY CMP0037 AND ${CMAKE_VERSION} VERSION_LESS "3.11.0") cmake_policy(SET CMP0037 OLD) endif() +# If this is a debug build, require libfiu. +if (CMAKE_BUILD_TYPE MATCHES "(DEBUG|Debug|debug)") + find_library(FIU_ENABLE fiu) + set (test_LIBS fiu) +endif (CMAKE_BUILD_TYPE MATCHES "(DEBUG|Debug|debug)") + include_directories (${CMAKE_SOURCE_DIR} ${CMAKE_SOURCE_DIR}/src ${CMAKE_SOURCE_DIR}/test @@ -21,9 +27,8 @@ add_custom_target (test ./run_all --verbose foreach (src_FILE ${test_SRCS}) add_executable (${src_FILE} "${src_FILE}.cpp" test.cpp) - target_link_libraries (${src_FILE} shared ${SHARED_LIBRARIES}) + target_link_libraries (${src_FILE} shared ${test_LIBS} ${SHARED_LIBRARIES}) endforeach (src_FILE) configure_file(run_all run_all COPYONLY) configure_file(problems problems COPYONLY) - diff --git a/test/fs.t.cpp b/test/fs.t.cpp index 72cc14b..c63a623 100644 --- a/test/fs.t.cpp +++ b/test/fs.t.cpp @@ -27,16 +27,22 @@ #include #include #include +#include +#include #include #include #include +#include +#include -int main (int, char**) +int test (UnitTest& t) { - UnitTest t (128); try { + //////////////////////////////////////////////////////////////////////////// + // Path tests + // Path (); Path p0; t.is (p0._data, "", "Path::Path"); @@ -136,6 +142,8 @@ int main (int, char**) tmp.create (); t.ok (tmp.exists (), "tmp dir created."); + //////////////////////////////////////////////////////////////////////////// + // File tests File::write ("tmp/file.t.txt", "This is a test\n"); File f6 ("tmp/file.t.txt"); t.ok (f6.size () == 15, "File::size tmp/file.t.txt good"); @@ -191,11 +199,70 @@ int main (int, char**) t.notok (File::copy ("tmp/does-not-exits.txt", "tmp/should-not-exists.txt"), "File::copy returns false if not exists"); t.notok (File ("tmp/should-not-exists.txt").exists (), "File::copy destination should not exist"); + //////////////////////////////////////////////////////////////////////////// + // AtomicFile tests + File::write ("tmp/file.t.txt", "This is a test\n"); + AtomicFile af1 ("tmp/file.t.txt"); + t.ok (af1.size () == 15, "File::size tmp/file.t.txt good"); + t.ok (af1.mode () & S_IRUSR, "File::mode tmp/file.t.txt good"); + t.ok (File::remove ("tmp/file.t.txt"), "File::remove tmp/file.t.txt good"); + + // operator (std::string) const; + t.is ((std::string) af1, "tmp/file.t.txt", "File::operator (std::string) const"); + + t.ok (File::create ("tmp/file.t.create"), "File::create tmp/file.t.create good"); + t.ok (File::remove ("tmp/file.t.create"), "File::remove tmp/file.t.create good"); + + // basename (std::string) const; + t.is (af1.name (), "file.t.txt", "File::basename tmp/file.t.txt --> file.t.txt"); + + // dirname (std::string) const; + t.is (af1.parent (), "tmp", "File::dirname tmp/file.t.txt --> tmp"); + + // bool rename (const std::string&); + AtomicFile af2 ("tmp/file.t.2.txt"); + af2.append ("something\n"); + af2.close (); + + t.ok (af2.rename ("tmp/file.t.3.txt"), "File::rename did not fail"); + t.is (af2._data, "tmp/file.t.3.txt", "File::rename stored new name"); + t.ok (af2.exists (), "File::rename new file exists"); + t.ok (File::remove ("tmp/file.t.3.txt"), "File::remove tmp/file.t.3.txt good"); + + // Test permissions. + AtomicFile af3 ("tmp/file.t.perm.txt"); + + File::write ("tmp/file.t.txt", "This is a test\n"); + t.ok (File::copy ("tmp/file.t.txt", "tmp/file.t.copy.txt"), "File::copy returned true"); + AtomicFile af4 ("tmp/file.t.copy.txt"); + t.ok (af4.exists (), "File::copy created copy"); + t.ok (af4.size () == 15, "File::copy tmp/file.t.copy.txt good after copy"); + t.notok (File::copy ("tmp/does-not-exits.txt", "tmp/should-not-exists.txt"), "File::copy returns false if not exists"); + t.notok (File ("tmp/should-not-exists.txt").exists (), "File::copy destination should not exist"); + + File::write ("tmp/file.5.txt", "This is a test\n"); + File::write ("tmp/file.5.txt.new", "This is a test\n"); + bool exception_thrown = false; + try { + AtomicFile af5; + af5 = AtomicFile ("tmp/file.5.txt"); + } + catch(...) { + exception_thrown = true; + } + t.ok (exception_thrown, "AtomicFile raises exception when .new file already exists"); + + + //////////////////////////////////////////////////////////////////////////// + // Path and *File tests cleanup: tmp.remove (); t.notok (tmp.exists (), "tmp dir removed."); tmp.create (); t.ok (tmp.exists (), "tmp dir created."); + //////////////////////////////////////////////////////////////////////////// + // Directory tests + // Directory (const File&); // Directory (const Path&); Directory d0 (Path ("tmp")); @@ -335,3 +402,97 @@ int main (int, char**) } //////////////////////////////////////////////////////////////////////////////// +// Fault injection tests - using https://github.com/albertito/libfiu + + +// This class is a helper to make sure we turn off libfiu after the test so that +// we can properly write to stdout / stderr. +class FIU +{ +public: + FIU() + { + enable(); + } + + FIU(const FIU&) = delete; + FIU& operator= (const FIU&) = delete; + + ~FIU() + { + disable (); + std::cout << cbuffer.str(); + std::stringstream().swap(cbuffer); + } + + void enable () + { + for (auto test_point : test_points) + { + fiu_enable_external(test_point, 1, NULL, 0, fiu_cb); + } + } + + void disable () + { + for (auto test_point : test_points) + { + fiu_disable(test_point); + } + } + +private: + static const std::vector test_points; + static std::stringstream cbuffer; + + static int external_cb_was_called; + + static int fiu_cb(const char *name, int *failnum, + void **failinfo, unsigned int *flags) + { + (void)name; + (void)flags; + external_cb_was_called++; + + // For debugging the tests themselves... + // cbuffer << "fiu_cb called for " << name << '\n'; + + *failinfo = (void *) EIO; + + return *failnum; + } +}; + +const std::vector FIU::test_points { + "posix/stdio/gp/fputs", + "posix/io/rw/write", +}; + +std::stringstream FIU::cbuffer; +int FIU::external_cb_was_called = 0; + +int fiu_test (UnitTest& t) +{ + Directory tmp ("tmp"); + tmp.create (); + fiu_init (0); + + try { + FIU fiu; + AtomicFile ("tmp/test.txt").write_raw ("This is test"); + t.fail ("AtomicFile::write_raw throws on error"); } + catch (...) { + t.pass ("AtomicFile::write_raw throws on error"); + } + + return 1; +} + +//////////////////////////////////////////////////////////////////////////////// + +int main (int, char**) +{ + UnitTest t (128); + + return test (t) + fiu_test (t); +} \ No newline at end of file