From efa3b32147d196f17ba6d60e942c08a253bd2963 Mon Sep 17 00:00:00 2001 From: Vladimir Rusinov Date: Thu, 19 Aug 2021 17:21:39 +0100 Subject: [PATCH 01/10] Create (currently empty) AbstractFile class. --- src/FS.cpp | 10 +++++----- src/FS.h | 10 ++++++++-- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/FS.cpp b/src/FS.cpp index 3aa9fec..7607725 100644 --- a/src/FS.cpp +++ b/src/FS.cpp @@ -900,25 +900,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 +926,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..8f5e7ae 100644 --- a/src/FS.h +++ b/src/FS.h @@ -70,7 +70,13 @@ class Path std::string _data; }; -class File : public Path +class AbstractFile : virtual public Path +{ +public: +private: +}; + +class File : public AbstractFile { public: File (); @@ -121,7 +127,7 @@ class File : public Path bool _locked; }; -class Directory : public File +class Directory : public AbstractFile { public: Directory (); From 63482b555c0cc8b7ff9c67cbc1c518509c3e7b24 Mon Sep 17 00:00:00 2001 From: Vladimir Rusinov Date: Thu, 19 Aug 2021 21:10:10 +0000 Subject: [PATCH 02/10] Implement empty AtomicFile. It currently defines calls necessary for compiling with TDB2 and simply proxies them to regular File. Some naive tests also added to make sure basics work. As part of this change Directory changed to be child of Path. Most of the File methods did not made sense for it. It compiles with TaskWarrior but other users may see missing methods. Most of shared methods should be moved to Path. --- src/FS.cpp | 129 ++++++++++++++++++++++++++++++++++++++++---------- src/FS.h | 42 +++++++++++++++- test/fs.t.cpp | 73 ++++++++++++++++++++++++++++ 3 files changed, 217 insertions(+), 27 deletions(-) diff --git a/src/FS.cpp b/src/FS.cpp index 7607725..fc9cdd9 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,85 @@ bool File::move (const std::string& from, const std::string& to) return false; } +//////////////////////////////////////////////////////////////////////////////// +AtomicFile::AtomicFile () +: Path::Path () +, _original_file (File ()) +{ +} + +//////////////////////////////////////////////////////////////////////////////// +AtomicFile::AtomicFile (const std::string& in) +: Path::Path (in) +, _original_file (File (in)) +{ +} + +//////////////////////////////////////////////////////////////////////////////// +AtomicFile& AtomicFile::operator= (const AtomicFile& other) +{ + if (this != &other) { + Path::operator= (other); + this->_original_file = File (other._data); + } + + return *this; +} + +//////////////////////////////////////////////////////////////////////////////// +void AtomicFile::truncate () +{ + _original_file.truncate (); +} + +//////////////////////////////////////////////////////////////////////////////// +size_t AtomicFile::size () const +{ + return _original_file.size (); +} + +//////////////////////////////////////////////////////////////////////////////// +void AtomicFile::close () +{ + _original_file.close (); +} + +//////////////////////////////////////////////////////////////////////////////// +void AtomicFile::read (std::vector & contents) +{ + _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) +{ + _original_file.append (lines); +} + +//////////////////////////////////////////////////////////////////////////////// +void AtomicFile::append (const std::string& line) +{ + _original_file.append (line); +} + +//////////////////////////////////////////////////////////////////////////////// +void AtomicFile::write_raw (const std::string& line) +{ + _original_file.write_raw (line); +} + //////////////////////////////////////////////////////////////////////////////// Directory::Directory () { diff --git a/src/FS.h b/src/FS.h index 8f5e7ae..d15664d 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&); @@ -73,6 +75,19 @@ class Path class AbstractFile : virtual public Path { public: + virtual void truncate () = 0; + + virtual bool open () = 0; + virtual void close () = 0; + + virtual bool lock () = 0; + + virtual void read (std::vector &) = 0; + virtual void append (const std::string&) = 0; + virtual void append (const std::vector &) = 0; + virtual void write_raw (const std::string&) = 0; + + virtual size_t size () const = 0; private: }; @@ -105,7 +120,6 @@ class File : public AbstractFile void truncate (); - virtual mode_t mode (); virtual size_t size () const; virtual time_t mtime () const; virtual time_t ctime () const; @@ -127,7 +141,31 @@ class File : public AbstractFile bool _locked; }; -class Directory : public AbstractFile +class AtomicFile : public AbstractFile +{ +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; +}; + +class Directory : public Path { public: Directory (); diff --git a/test/fs.t.cpp b/test/fs.t.cpp index 72cc14b..eb1ae2e 100644 --- a/test/fs.t.cpp +++ b/test/fs.t.cpp @@ -37,6 +37,9 @@ int main (int, char**) try { + //////////////////////////////////////////////////////////////////////////// + // Path tests + // Path (); Path p0; t.is (p0._data, "", "Path::Path"); @@ -136,6 +139,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 +196,79 @@ 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"); + // remove () not implemented (yet?) + //t.ok (af2.remove (), "File::remove tmp/file.t.3.txt good"); + //t.notok (af2.exists (), "File::remove new file no longer 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"); + // create() not implemented (yet?) + // af3.create (0744); + // t.ok (af3.exists (), "File::create perm file exists"); + // mode_t m3 = af3.mode (); + // t.ok (m3 & S_IFREG, "File::mode tmp/file.t.perm.txt S_IFREG good"); + // t.ok (m3 & S_IRUSR, "File::mode tmp/file.t.perm.txt r-------- good"); + // t.ok (m3 & S_IWUSR, "File::mode tmp/file.t.perm.txt -w------- good"); + // t.ok (m3 & S_IXUSR, "File::mode tmp/file.t.perm.txt --x------ good"); + // t.ok (m3 & S_IRGRP, "File::mode tmp/file.t.perm.txt ---r----- good"); + // t.notok (m3 & S_IWGRP, "File::mode tmp/file.t.perm.txt ----w---- good"); + // t.notok (m3 & S_IXGRP, "File::mode tmp/file.t.perm.txt -----x--- good"); + // t.ok (m3 & S_IROTH, "File::mode tmp/file.t.perm.txt ------r-- good"); + // t.notok (m3 & S_IWOTH, "File::mode tmp/file.t.perm.txt -------w- good"); + // t.notok (m3 & S_IXOTH, "File::mode tmp/file.t.perm.txt --------x good"); + // remove () not implemented (yet?) + // af3.remove (); + // t.notok (af3.exists (), "File::remove perm file no longer exists"); + // t.ok (File::remove ("tmp/file.t.txt"), "File::remove tmp/file.t.txt good"); + + 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"); + + + //////////////////////////////////////////////////////////////////////////// + // 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")); From 15f48633df382edaccaeedb21b661a20baac436b Mon Sep 17 00:00:00 2001 From: Vladimir Rusinov Date: Thu, 19 Aug 2021 21:16:53 +0000 Subject: [PATCH 03/10] Add comment to AtomicFile --- src/FS.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/FS.h b/src/FS.h index d15664d..8542424 100644 --- a/src/FS.h +++ b/src/FS.h @@ -141,6 +141,15 @@ class File : public AbstractFile bool _locked; }; +// AtomicFile class. +// Currently just a very thin wrapper on top of File. +// The intent is to implement atomic file replace: instead of truncate + write +// as currently implemented by TDB2 + File, we will be creatin a new file and do +// a rename(). It would not solve all problems but will still make data loss on +// crash less likely. +// +// See discussion in +// https://github.com/GothenburgBitFactory/taskwarrior/issues/152 class AtomicFile : public AbstractFile { public: From 9b1e0d43bf7e93522d75907e5f23f0e9bd084ff9 Mon Sep 17 00:00:00 2001 From: Vladimir Rusinov Date: Fri, 20 Aug 2021 11:54:09 +0000 Subject: [PATCH 04/10] AtomicFile: ensure .new file does not already exist. Nothing creates .new file yet so this should be no-op. --- src/FS.cpp | 18 ++++++++++++++++++ src/FS.h | 5 +++++ test/fs.t.cpp | 12 ++++++++++++ 3 files changed, 35 insertions(+) diff --git a/src/FS.cpp b/src/FS.cpp index fc9cdd9..ed526bb 100644 --- a/src/FS.cpp +++ b/src/FS.cpp @@ -897,6 +897,7 @@ bool File::move (const std::string& from, const std::string& to) AtomicFile::AtomicFile () : Path::Path () , _original_file (File ()) +, _new_file (File ()) { } @@ -904,7 +905,9 @@ AtomicFile::AtomicFile () AtomicFile::AtomicFile (const std::string& in) : Path::Path (in) , _original_file (File (in)) +, _new_file (File (in + ".new")) { + assert_no_new_file (); } //////////////////////////////////////////////////////////////////////////////// @@ -913,6 +916,9 @@ 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; @@ -972,6 +978,18 @@ void AtomicFile::write_raw (const std::string& line) _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 () { diff --git a/src/FS.h b/src/FS.h index 8542424..8f43773 100644 --- a/src/FS.h +++ b/src/FS.h @@ -172,6 +172,11 @@ class AtomicFile : public AbstractFile size_t size () const; private: File _original_file; + File _new_file; + + // Ensures .new file does not exists. + // throws exception if it does. + void assert_no_new_file (); }; class Directory : public Path diff --git a/test/fs.t.cpp b/test/fs.t.cpp index eb1ae2e..af82b5f 100644 --- a/test/fs.t.cpp +++ b/test/fs.t.cpp @@ -258,6 +258,18 @@ 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"); + 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: From 9cbd8ebd77b5bae1f0241c77bc19e1ce6fa29055 Mon Sep 17 00:00:00 2001 From: Vladimir Rusinov Date: Fri, 20 Aug 2021 15:05:42 +0000 Subject: [PATCH 05/10] Working version / proof of concept of AtomicFile. --- src/FS.cpp | 57 +++++++++++++++++++++++++++++++++++++++++++++++++----- src/FS.h | 15 +++++++++----- 2 files changed, 62 insertions(+), 10 deletions(-) diff --git a/src/FS.cpp b/src/FS.cpp index ed526bb..62cd30d 100644 --- a/src/FS.cpp +++ b/src/FS.cpp @@ -898,6 +898,7 @@ AtomicFile::AtomicFile () : Path::Path () , _original_file (File ()) , _new_file (File ()) +, _new_file_in_use (false) { } @@ -906,6 +907,7 @@ 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 (); } @@ -927,7 +929,13 @@ AtomicFile& AtomicFile::operator= (const AtomicFile& other) //////////////////////////////////////////////////////////////////////////////// void AtomicFile::truncate () { - _original_file.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; } //////////////////////////////////////////////////////////////////////////////// @@ -939,12 +947,30 @@ size_t AtomicFile::size () const //////////////////////////////////////////////////////////////////////////////// void AtomicFile::close () { - _original_file.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); } @@ -963,19 +989,40 @@ bool AtomicFile::open () //////////////////////////////////////////////////////////////////////////////// void AtomicFile::append (const std::vector & lines) { - _original_file.append (lines); + if (_new_file_in_use) + { + _new_file.append (lines); + } + else + { + _original_file.append (lines); + } } //////////////////////////////////////////////////////////////////////////////// void AtomicFile::append (const std::string& line) { - _original_file.append (line); + if (_new_file_in_use) + { + _new_file.append (line); + } + else + { + _original_file.append (line); + } } //////////////////////////////////////////////////////////////////////////////// void AtomicFile::write_raw (const std::string& line) { - _original_file.write_raw (line); + if (_new_file_in_use) + { + _new_file.write_raw (line); + } + else + { + _original_file.write_raw (line); + } } //////////////////////////////////////////////////////////////////////////////// diff --git a/src/FS.h b/src/FS.h index 8f43773..a4ca4e4 100644 --- a/src/FS.h +++ b/src/FS.h @@ -142,11 +142,15 @@ class File : public AbstractFile }; // AtomicFile class. -// Currently just a very thin wrapper on top of File. -// The intent is to implement atomic file replace: instead of truncate + write -// as currently implemented by TDB2 + File, we will be creatin a new file and do -// a rename(). It would not solve all problems but will still make data loss on -// crash less likely. +// 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 @@ -173,6 +177,7 @@ class AtomicFile : public AbstractFile private: File _original_file; File _new_file; + bool _new_file_in_use; // Ensures .new file does not exists. // throws exception if it does. From 20521e6497776d2693881ba645faa2ab3f259614 Mon Sep 17 00:00:00 2001 From: Vladimir Rusinov Date: Mon, 11 Oct 2021 14:56:13 +0100 Subject: [PATCH 06/10] Remove AtomicFile interface. Does not seem to be that useful. --- src/FS.h | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/src/FS.h b/src/FS.h index a4ca4e4..141ea7b 100644 --- a/src/FS.h +++ b/src/FS.h @@ -72,26 +72,7 @@ class Path std::string _data; }; -class AbstractFile : virtual public Path -{ -public: - virtual void truncate () = 0; - - virtual bool open () = 0; - virtual void close () = 0; - - virtual bool lock () = 0; - - virtual void read (std::vector &) = 0; - virtual void append (const std::string&) = 0; - virtual void append (const std::vector &) = 0; - virtual void write_raw (const std::string&) = 0; - - virtual size_t size () const = 0; -private: -}; - -class File : public AbstractFile +class File : public Path { public: File (); @@ -154,7 +135,7 @@ class File : public AbstractFile // // See discussion in // https://github.com/GothenburgBitFactory/taskwarrior/issues/152 -class AtomicFile : public AbstractFile +class AtomicFile : public Path { public: AtomicFile (); From e27fe1773a8f7fe6e89439d3821b8788826a1817 Mon Sep 17 00:00:00 2001 From: Vladimir Rusinov Date: Thu, 14 Oct 2021 10:07:56 +0000 Subject: [PATCH 07/10] Remove unnecessary commented out AtomicFile tests --- test/fs.t.cpp | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/test/fs.t.cpp b/test/fs.t.cpp index af82b5f..cd80b21 100644 --- a/test/fs.t.cpp +++ b/test/fs.t.cpp @@ -224,31 +224,10 @@ int main (int, char**) 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"); - // remove () not implemented (yet?) - //t.ok (af2.remove (), "File::remove tmp/file.t.3.txt good"); - //t.notok (af2.exists (), "File::remove new file no longer 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"); - // create() not implemented (yet?) - // af3.create (0744); - // t.ok (af3.exists (), "File::create perm file exists"); - // mode_t m3 = af3.mode (); - // t.ok (m3 & S_IFREG, "File::mode tmp/file.t.perm.txt S_IFREG good"); - // t.ok (m3 & S_IRUSR, "File::mode tmp/file.t.perm.txt r-------- good"); - // t.ok (m3 & S_IWUSR, "File::mode tmp/file.t.perm.txt -w------- good"); - // t.ok (m3 & S_IXUSR, "File::mode tmp/file.t.perm.txt --x------ good"); - // t.ok (m3 & S_IRGRP, "File::mode tmp/file.t.perm.txt ---r----- good"); - // t.notok (m3 & S_IWGRP, "File::mode tmp/file.t.perm.txt ----w---- good"); - // t.notok (m3 & S_IXGRP, "File::mode tmp/file.t.perm.txt -----x--- good"); - // t.ok (m3 & S_IROTH, "File::mode tmp/file.t.perm.txt ------r-- good"); - // t.notok (m3 & S_IWOTH, "File::mode tmp/file.t.perm.txt -------w- good"); - // t.notok (m3 & S_IXOTH, "File::mode tmp/file.t.perm.txt --------x good"); - // remove () not implemented (yet?) - // af3.remove (); - // t.notok (af3.exists (), "File::remove perm file no longer exists"); - // t.ok (File::remove ("tmp/file.t.txt"), "File::remove tmp/file.t.txt good"); 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"); From 7c642535b2858a509fa47497cef7bd6f12dffd70 Mon Sep 17 00:00:00 2001 From: Vladimir Rusinov Date: Thu, 14 Oct 2021 11:51:40 +0000 Subject: [PATCH 08/10] Create separate function for test. Will be useful later when adding fiu tests. --- test/fs.t.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/test/fs.t.cpp b/test/fs.t.cpp index cd80b21..65c2374 100644 --- a/test/fs.t.cpp +++ b/test/fs.t.cpp @@ -31,9 +31,8 @@ #include #include -int main (int, char**) +int test (UnitTest& t) { - UnitTest t (128); try { @@ -399,3 +398,11 @@ int main (int, char**) } //////////////////////////////////////////////////////////////////////////////// + +int main (int, char**) +{ + UnitTest t (128); + + test(t); + +} \ No newline at end of file From 9d1da2a97e2239bfbf8719e62b94542706a1f9b6 Mon Sep 17 00:00:00 2001 From: Vladimir Rusinov Date: Thu, 14 Oct 2021 12:04:23 +0000 Subject: [PATCH 09/10] Create (currently empty) fiu_test --- test/fs.t.cpp | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/test/fs.t.cpp b/test/fs.t.cpp index 65c2374..0a7ff94 100644 --- a/test/fs.t.cpp +++ b/test/fs.t.cpp @@ -397,12 +397,21 @@ int test (UnitTest& t) return 0; } +//////////////////////////////////////////////////////////////////////////////// +// Fault injection tests - using https://github.com/albertito/libfiu + + +int fiu_test (UnitTest& t) +{ + t.skip ("fiu test is not implemented yet"); + return 1; +} + //////////////////////////////////////////////////////////////////////////////// int main (int, char**) { UnitTest t (128); - test(t); - + return test (t) + fiu_test (t); } \ No newline at end of file From aad5b452c8594b79b09c6e1a1c830713e7c239ef Mon Sep 17 00:00:00 2001 From: Vladimir Rusinov Date: Wed, 20 Apr 2022 10:14:02 +0100 Subject: [PATCH 10/10] Add FIU tests --- test/CMakeLists.txt | 9 +++-- test/fs.t.cpp | 83 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 89 insertions(+), 3 deletions(-) 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 0a7ff94..c63a623 100644 --- a/test/fs.t.cpp +++ b/test/fs.t.cpp @@ -27,9 +27,13 @@ #include #include #include +#include +#include #include #include #include +#include +#include int test (UnitTest& t) { @@ -401,9 +405,86 @@ int test (UnitTest& t) // 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) { - t.skip ("fiu test is not implemented yet"); + 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; }