From 01dece6cd0d1b4e8dde7e0eed00612df818d978b Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Thu, 13 Sep 2018 20:45:31 -0700 Subject: [PATCH 1/3] Decouple stat. Dokan in Windows and Fuse in Linux use different structs for that. --- src/cryfs/filesystem/CryNode.cpp | 24 ++++--- src/cryfs/filesystem/CryNode.h | 2 +- src/cryfs/filesystem/CryOpenFile.cpp | 5 +- src/cryfs/filesystem/CryOpenFile.h | 2 +- .../cachingfsblobstore/DirBlobRef.h | 9 +-- src/cryfs/filesystem/fsblobstore/DirBlob.cpp | 28 ++++---- src/cryfs/filesystem/fsblobstore/DirBlob.h | 5 +- .../parallelaccessfsblobstore/DirBlobRef.h | 9 +-- src/fspp/fs_interface/Node.h | 16 ++++- src/fspp/fs_interface/OpenFile.h | 6 +- src/fspp/fstest/FsppDeviceTest_Timestamps.h | 10 +-- src/fspp/fstest/FsppFileTest.h | 14 ++-- src/fspp/fstest/FsppNodeTest_Stat.h | 16 ++--- src/fspp/fstest/FsppNodeTest_Timestamps.h | 13 ++-- src/fspp/fstest/FsppOpenFileTest.h | 13 ++-- src/fspp/fstest/FsppOpenFileTest_Timestamps.h | 7 +- src/fspp/fstest/testutils/FileSystemTest.h | 9 ++- src/fspp/fstest/testutils/FileTest.h | 23 +++--- src/fspp/fstest/testutils/FsppNodeTest.h | 9 ++- .../fstest/testutils/TimestampTestUtils.h | 72 +++++++++---------- src/fspp/impl/FilesystemImpl.cpp | 20 +++++- test/fspp/impl/FuseOpenFileListTest.cpp | 2 +- 22 files changed, 169 insertions(+), 145 deletions(-) diff --git a/src/cryfs/filesystem/CryNode.cpp b/src/cryfs/filesystem/CryNode.cpp index 43c3aade..db2830ce 100644 --- a/src/cryfs/filesystem/CryNode.cpp +++ b/src/cryfs/filesystem/CryNode.cpp @@ -157,23 +157,25 @@ const blockstore::BlockId &CryNode::blockId() const { return _blockId; } -void CryNode::stat(struct ::stat *result) const { +CryNode::stat_info CryNode::stat() const { device()->callFsActionCallbacks(); if(_parent == none) { + stat_info result; //We are the root directory. - //TODO What should we do? - result->st_uid = getuid(); - result->st_gid = getgid(); - result->st_mode = S_IFDIR | S_IRUSR | S_IWUSR | S_IXUSR; - result->st_size = fsblobstore::DirBlob::DIR_LSTAT_SIZE; + //TODO What should we do? + result.uid = getuid(); + result.gid = getgid(); + result.mode = S_IFDIR | S_IRUSR | S_IWUSR | S_IXUSR; + result.size = fsblobstore::DirBlob::DIR_LSTAT_SIZE; //TODO If possible without performance loss, then for a directory, st_nlink should return number of dir entries (including "." and "..") - result->st_nlink = 1; + result.nlink = 1; struct timespec now = cpputils::time::now(); - result->st_atim = now; - result->st_mtim = now; - result->st_ctim = now; + result.atime = now; + result.mtime = now; + result.ctime = now; + return result; } else { - (*_parent)->statChild(_blockId, result); + return (*_parent)->statChild(_blockId); } } diff --git a/src/cryfs/filesystem/CryNode.h b/src/cryfs/filesystem/CryNode.h index 2e362ca1..bb11040f 100644 --- a/src/cryfs/filesystem/CryNode.h +++ b/src/cryfs/filesystem/CryNode.h @@ -17,7 +17,7 @@ public: // TODO grandparent is only needed to set the timestamps of the parent directory on rename and remove. Delete grandparent parameter once we store timestamps in the blob itself instead of in the directory listing. CryNode(CryDevice *device, boost::optional> parent, boost::optional> grandparent, const blockstore::BlockId &blockId); void access(int mask) const override; - void stat(struct ::stat *result) const override; + stat_info stat() const override; void chmod(mode_t mode) override; void chown(uid_t uid, gid_t gid) override; void rename(const boost::filesystem::path &to) override; diff --git a/src/cryfs/filesystem/CryOpenFile.cpp b/src/cryfs/filesystem/CryOpenFile.cpp index 6c78661e..194f0128 100644 --- a/src/cryfs/filesystem/CryOpenFile.cpp +++ b/src/cryfs/filesystem/CryOpenFile.cpp @@ -30,10 +30,9 @@ void CryOpenFile::flush() { _parent->flush(); } -void CryOpenFile::stat(struct ::stat *result) const { +fspp::Node::stat_info CryOpenFile::stat() const { _device->callFsActionCallbacks(); - result->st_size = _fileBlob->size(); - _parent->statChildWithSizeAlreadySet(_fileBlob->blockId(), result); + return _parent->statChildWithKnownSize(_fileBlob->blockId(), _fileBlob->size()); } void CryOpenFile::truncate(off_t size) const { diff --git a/src/cryfs/filesystem/CryOpenFile.h b/src/cryfs/filesystem/CryOpenFile.h index b483f584..f34dea50 100644 --- a/src/cryfs/filesystem/CryOpenFile.h +++ b/src/cryfs/filesystem/CryOpenFile.h @@ -14,7 +14,7 @@ public: explicit CryOpenFile(const CryDevice *device, std::shared_ptr parent, cpputils::unique_ref fileBlob); ~CryOpenFile(); - void stat(struct ::stat *result) const override; + stat_info stat() const override; void truncate(off_t size) const override; size_t read(void *buf, size_t count, off_t offset) const override; void write(const void *buf, size_t count, off_t offset) override; diff --git a/src/cryfs/filesystem/cachingfsblobstore/DirBlobRef.h b/src/cryfs/filesystem/cachingfsblobstore/DirBlobRef.h index 9fe4f448..b50510cb 100644 --- a/src/cryfs/filesystem/cachingfsblobstore/DirBlobRef.h +++ b/src/cryfs/filesystem/cachingfsblobstore/DirBlobRef.h @@ -5,6 +5,7 @@ #include "FsBlobRef.h" #include "../fsblobstore/DirBlob.h" #include "../fsblobstore/utils/TimestampUpdateBehavior.h" +#include namespace cryfs { namespace cachingfsblobstore { @@ -53,12 +54,12 @@ public: return _base->RenameChild(blockId, newName, onOverwritten); } - void statChild(const blockstore::BlockId &blockId, struct ::stat *result) const { - return _base->statChild(blockId, result); + fspp::Node::stat_info statChild(const blockstore::BlockId &blockId) const { + return _base->statChild(blockId); } - void statChildWithSizeAlreadySet(const blockstore::BlockId &blockId, struct ::stat *result) const { - return _base->statChildWithSizeAlreadySet(blockId, result); + fspp::Node::stat_info statChildWithKnownSize(const blockstore::BlockId &blockId, uint64_t size) const { + return _base->statChildWithKnownSize(blockId, size); } void updateAccessTimestampForChild(const blockstore::BlockId &blockId, fsblobstore::TimestampUpdateBehavior timestampUpdateBehavior) { diff --git a/src/cryfs/filesystem/fsblobstore/DirBlob.cpp b/src/cryfs/filesystem/fsblobstore/DirBlob.cpp index a7928e70..046792f9 100644 --- a/src/cryfs/filesystem/fsblobstore/DirBlob.cpp +++ b/src/cryfs/filesystem/fsblobstore/DirBlob.cpp @@ -132,28 +132,30 @@ off_t DirBlob::lstat_size() const { return DIR_LSTAT_SIZE; } -void DirBlob::statChild(const BlockId &blockId, struct ::stat *result) const { - result->st_size = _getLstatSize(blockId); - statChildWithSizeAlreadySet(blockId, result); +fspp::Node::stat_info DirBlob::statChild(const BlockId &blockId) const { + return statChildWithKnownSize(blockId, _getLstatSize(blockId)); } -void DirBlob::statChildWithSizeAlreadySet(const BlockId &blockId, struct ::stat *result) const { +fspp::Node::stat_info DirBlob::statChildWithKnownSize(const BlockId &blockId, uint64_t size) const { + fspp::Node::stat_info result; + auto childOpt = GetChild(blockId); if (childOpt == boost::none) { throw fspp::fuse::FuseErrnoException(ENOENT); } const auto &child = *childOpt; - result->st_mode = child.mode(); - result->st_uid = child.uid(); - result->st_gid = child.gid(); + result.mode = child.mode(); + result.uid = child.uid(); + result.gid = child.gid(); //TODO If possible without performance loss, then for a directory, st_nlink should return number of dir entries (including "." and "..") - result->st_nlink = 1; - result->st_atim = child.lastAccessTime(); - result->st_mtim = child.lastModificationTime(); - result->st_ctim = child.lastMetadataChangeTime(); + result.nlink = 1; + result.size = size; + result.atime = child.lastAccessTime(); + result.mtime = child.lastModificationTime(); + result.ctime = child.lastMetadataChangeTime(); //TODO Move ceilDivision to general utils which can be used by cryfs as well - result->st_blocks = blobstore::onblocks::utils::ceilDivision(result->st_size, static_cast(512)); - result->st_blksize = _fsBlobStore->virtualBlocksizeBytes(); + result.blocks = blobstore::onblocks::utils::ceilDivision(size, static_cast(512)); + return result; } void DirBlob::updateAccessTimestampForChild(const BlockId &blockId, TimestampUpdateBehavior timestampUpdateBehavior) { diff --git a/src/cryfs/filesystem/fsblobstore/DirBlob.h b/src/cryfs/filesystem/fsblobstore/DirBlob.h index a80c99b5..1dcb2e84 100644 --- a/src/cryfs/filesystem/fsblobstore/DirBlob.h +++ b/src/cryfs/filesystem/fsblobstore/DirBlob.h @@ -5,6 +5,7 @@ #include #include #include +#include #include "FsBlob.h" #include "utils/DirEntryList.h" #include @@ -56,9 +57,9 @@ namespace cryfs { void flush(); - void statChild(const blockstore::BlockId &blockId, struct ::stat *result) const; + fspp::Node::stat_info statChild(const blockstore::BlockId &blockId) const; - void statChildWithSizeAlreadySet(const blockstore::BlockId &blockId, struct ::stat *result) const; + fspp::Node::stat_info statChildWithKnownSize(const blockstore::BlockId &blockId, uint64_t size) const; void updateAccessTimestampForChild(const blockstore::BlockId &blockId, TimestampUpdateBehavior timestampUpdateBehavior); diff --git a/src/cryfs/filesystem/parallelaccessfsblobstore/DirBlobRef.h b/src/cryfs/filesystem/parallelaccessfsblobstore/DirBlobRef.h index 039bcea9..cd83700f 100644 --- a/src/cryfs/filesystem/parallelaccessfsblobstore/DirBlobRef.h +++ b/src/cryfs/filesystem/parallelaccessfsblobstore/DirBlobRef.h @@ -5,6 +5,7 @@ #include "FsBlobRef.h" #include "../cachingfsblobstore/DirBlobRef.h" #include "../fsblobstore/utils/TimestampUpdateBehavior.h" +#include namespace cryfs { namespace parallelaccessfsblobstore { @@ -49,12 +50,12 @@ public: return _base->RenameChild(blockId, newName, onOverwritten); } - void statChild(const blockstore::BlockId &blockId, struct ::stat *result) const { - return _base->statChild(blockId, result); + fspp::Node::stat_info statChild(const blockstore::BlockId &blockId) const { + return _base->statChild(blockId); } - void statChildWithSizeAlreadySet(const blockstore::BlockId &blockId, struct ::stat *result) const { - return _base->statChildWithSizeAlreadySet(blockId, result); + fspp::Node::stat_info statChildWithKnownSize(const blockstore::BlockId &blockId, uint64_t size) const { + return _base->statChildWithKnownSize(blockId, size); } void updateAccessTimestampForChild(const blockstore::BlockId &blockId, fsblobstore::TimestampUpdateBehavior timestampUpdateBehavior) { diff --git a/src/fspp/fs_interface/Node.h b/src/fspp/fs_interface/Node.h index aae0866a..c00c7108 100644 --- a/src/fspp/fs_interface/Node.h +++ b/src/fspp/fs_interface/Node.h @@ -4,15 +4,25 @@ #include -#include - namespace fspp { class Node { public: virtual ~Node() {} - virtual void stat(struct ::stat *result) const = 0; + struct stat_info final { + uint32_t nlink; + uint32_t mode; + uint32_t uid; + uint32_t gid; + uint64_t size; + uint64_t blocks; + struct timespec atime; + struct timespec mtime; + struct timespec ctime; + }; + + virtual stat_info stat() const = 0; virtual void chmod(mode_t mode) = 0; virtual void chown(uid_t uid, gid_t gid) = 0; virtual void access(int mask) const = 0; diff --git a/src/fspp/fs_interface/OpenFile.h b/src/fspp/fs_interface/OpenFile.h index 4f1b464c..f9ad54c0 100644 --- a/src/fspp/fs_interface/OpenFile.h +++ b/src/fspp/fs_interface/OpenFile.h @@ -3,7 +3,7 @@ #define MESSMER_FSPP_FSINTERFACE_OPENFILE_H_ #include -#include +#include "Node.h" namespace fspp { class Device; @@ -12,7 +12,9 @@ class OpenFile { public: virtual ~OpenFile() {} - virtual void stat(struct ::stat *result) const = 0; + using stat_info = typename Node::stat_info; + + virtual stat_info stat() const = 0; virtual void truncate(off_t size) const = 0; virtual size_t read(void *buf, size_t count, off_t offset) const = 0; virtual void write(const void *buf, size_t count, off_t offset) = 0; diff --git a/src/fspp/fstest/FsppDeviceTest_Timestamps.h b/src/fspp/fstest/FsppDeviceTest_Timestamps.h index d3d38367..c03d1c99 100644 --- a/src/fspp/fstest/FsppDeviceTest_Timestamps.h +++ b/src/fspp/fstest/FsppDeviceTest_Timestamps.h @@ -16,7 +16,7 @@ public: } void Test_Load_While_Not_Loaded() { - struct stat oldStat{}; + fspp::Node::stat_info oldStat{}; { auto node = this->CreateNode("/mynode"); oldStat = this->stat(*node); @@ -28,10 +28,10 @@ public: auto node = this->device->Load("/mynode"); //Test that timestamps didn't change - struct stat newStat = this->stat(*node.value()); - EXPECT_EQ(oldStat.st_atim, newStat.st_atim); - EXPECT_EQ(oldStat.st_mtim, newStat.st_mtim); - EXPECT_EQ(oldStat.st_ctim, newStat.st_ctim); + fspp::Node::stat_info newStat = this->stat(*node.value()); + EXPECT_EQ(oldStat.atime, newStat.atime); + EXPECT_EQ(oldStat.mtime, newStat.mtime); + EXPECT_EQ(oldStat.ctime, newStat.ctime); } }; diff --git a/src/fspp/fstest/FsppFileTest.h b/src/fspp/fstest/FsppFileTest.h index 95538676..e5ab1364 100644 --- a/src/fspp/fstest/FsppFileTest.h +++ b/src/fspp/fstest/FsppFileTest.h @@ -58,22 +58,22 @@ public: void Test_Chown_Uid(fspp::File *file, fspp::Node *node) { node->chown(100, 200); - this->IN_STAT(file, node, [] (struct stat st){ - EXPECT_EQ(100u, st.st_uid); + this->IN_STAT(file, node, [] (const fspp::Node::stat_info& st){ + EXPECT_EQ(100u, st.uid); }); } void Test_Chown_Gid(fspp::File *file, fspp::Node *node) { node->chown(100, 200); - this->IN_STAT(file, node, [] (struct stat st){ - EXPECT_EQ(200u, st.st_gid); + this->IN_STAT(file, node, [] (const fspp::Node::stat_info& st){ + EXPECT_EQ(200u, st.gid); }); } void Test_Chmod(fspp::File *file, fspp::Node *node) { node->chmod(S_IFREG | S_IRUSR | S_IWOTH); - this->IN_STAT(file, node, [] (struct stat st){ - EXPECT_EQ(static_cast(S_IFREG | S_IRUSR | S_IWOTH), st.st_mode); + this->IN_STAT(file, node, [] (const fspp::Node::stat_info& st){ + EXPECT_EQ(static_cast(S_IFREG | S_IRUSR | S_IWOTH), st.mode); }); } @@ -81,7 +81,7 @@ public: struct timespec ATIME{}; ATIME.tv_sec = 1458086400; ATIME.tv_nsec = 34525; struct timespec MTIME{}; MTIME.tv_sec = 1458086300; MTIME.tv_nsec = 48293; node->utimens(ATIME, MTIME); - this->IN_STAT(file, node, [this, ATIME, MTIME] (struct stat st) { + this->IN_STAT(file, node, [this, ATIME, MTIME] (const fspp::Node::stat_info& st) { this->EXPECT_ATIME_EQ(ATIME, st); this->EXPECT_MTIME_EQ(MTIME, st); }); diff --git a/src/fspp/fstest/FsppNodeTest_Stat.h b/src/fspp/fstest/FsppNodeTest_Stat.h index 69fcdb11..41c4d00f 100644 --- a/src/fspp/fstest/FsppNodeTest_Stat.h +++ b/src/fspp/fstest/FsppNodeTest_Stat.h @@ -11,8 +11,8 @@ public: void Test_Nlink() { this->CreateNode("/mynode"); auto node = this->Load("/mynode"); - this->IN_STAT(node.get(), [] (struct stat st) { - EXPECT_EQ(1u, st.st_nlink); + this->IN_STAT(node.get(), [] (const fspp::Node::stat_info& st) { + EXPECT_EQ(1u, st.nlink); }); } }; @@ -32,8 +32,8 @@ TYPED_TEST_P(FsppNodeTest_Stat_FileOnly, CreatedFileIsEmpty) { TYPED_TEST_P(FsppNodeTest_Stat_FileOnly, FileIsFile) { this->CreateFile("/myfile"); auto node = this->Load("/myfile"); - this->IN_STAT(node.get(), [] (struct stat st) { - EXPECT_TRUE(S_ISREG(st.st_mode)); + this->IN_STAT(node.get(), [] (const fspp::Node::stat_info& st) { + EXPECT_TRUE(S_ISREG(st.mode)); }); } @@ -46,8 +46,8 @@ TYPED_TEST_CASE_P(FsppNodeTest_Stat_DirOnly); TYPED_TEST_P(FsppNodeTest_Stat_DirOnly, DirIsDir) { this->CreateDir("/mydir"); auto node = this->Load("/mydir"); - this->IN_STAT(node.get(), [] (struct stat st) { - EXPECT_TRUE(S_ISDIR(st.st_mode)); + this->IN_STAT(node.get(), [] (const fspp::Node::stat_info& st) { + EXPECT_TRUE(S_ISDIR(st.mode)); }); } @@ -60,8 +60,8 @@ TYPED_TEST_CASE_P(FsppNodeTest_Stat_SymlinkOnly); TYPED_TEST_P(FsppNodeTest_Stat_SymlinkOnly, SymlinkIsSymlink) { this->CreateSymlink("/mysymlink"); auto node = this->Load("/mysymlink"); - this->IN_STAT(node.get(), [] (struct stat st) { - EXPECT_TRUE(S_ISLNK(st.st_mode)); + this->IN_STAT(node.get(), [] (const fspp::Node::stat_info& st) { + EXPECT_TRUE(S_ISLNK(st.mode)); }); } diff --git a/src/fspp/fstest/FsppNodeTest_Timestamps.h b/src/fspp/fstest/FsppNodeTest_Timestamps.h index 48e0fe52..34a0baf7 100644 --- a/src/fspp/fstest/FsppNodeTest_Timestamps.h +++ b/src/fspp/fstest/FsppNodeTest_Timestamps.h @@ -26,8 +26,7 @@ public: void Test_Stat() { auto node = this->CreateNode("/mynode"); auto operation = [&node] () { - struct stat st{}; - node->stat(&st); + node->stat(); }; this->EXPECT_OPERATION_UPDATES_TIMESTAMPS_AS("/mynode", operation, { this->ExpectDoesntUpdateAnyTimestamps @@ -36,7 +35,7 @@ public: void Test_Chmod() { auto node = this->CreateNode("/mynode"); - mode_t mode = this->stat(*node).st_mode; + mode_t mode = this->stat(*node).mode; auto operation = [&node, mode] () { node->chmod(mode); }; @@ -49,8 +48,8 @@ public: void Test_Chown() { auto node = this->CreateNode("/mynode"); - uid_t uid = this->stat(*node).st_uid; - gid_t gid = this->stat(*node).st_gid; + uid_t uid = this->stat(*node).uid; + gid_t gid = this->stat(*node).gid; auto operation = [&node, uid, gid] () { node->chown(uid, gid); }; @@ -347,8 +346,8 @@ public: this->EXPECT_OPERATION_UPDATES_TIMESTAMPS_AS("/mynode", operation, { this->ExpectUpdatesMetadataTimestamp }); - EXPECT_EQ(atime, this->stat(*node).st_atim); - EXPECT_EQ(mtime, this->stat(*node).st_mtim); + EXPECT_EQ(atime, this->stat(*node).atime); + EXPECT_EQ(mtime, this->stat(*node).mtime); } }; diff --git a/src/fspp/fstest/FsppOpenFileTest.h b/src/fspp/fstest/FsppOpenFileTest.h index 3c2f7bbb..260c8358 100644 --- a/src/fspp/fstest/FsppOpenFileTest.h +++ b/src/fspp/fstest/FsppOpenFileTest.h @@ -7,15 +7,14 @@ template class FsppOpenFileTest: public FileSystemTest { public: - void IN_STAT(fspp::OpenFile *openFile, std::function callback) { - struct stat st{}; - openFile->stat(&st); + void IN_STAT(fspp::OpenFile *openFile, std::function callback) { + auto st = openFile->stat(); callback(st); } void EXPECT_SIZE(uint64_t expectedSize, fspp::OpenFile *openFile) { - IN_STAT(openFile, [expectedSize] (struct stat st) { - EXPECT_EQ(expectedSize, static_cast(st.st_size)); + IN_STAT(openFile, [expectedSize] (const fspp::OpenFile::stat_info& st) { + EXPECT_EQ(expectedSize, static_cast(st.size)); }); EXPECT_NUMBYTES_READABLE(expectedSize, openFile); @@ -41,8 +40,8 @@ TYPED_TEST_P(FsppOpenFileTest, CreatedFileIsEmpty) { TYPED_TEST_P(FsppOpenFileTest, FileIsFile) { auto file = this->CreateFile("/myfile"); auto openFile = this->LoadFile("/myfile")->open(O_RDONLY); - this->IN_STAT(openFile.get(), [] (struct stat st) { - EXPECT_TRUE(S_ISREG(st.st_mode)); + this->IN_STAT(openFile.get(), [] (const fspp::OpenFile::stat_info& st) { + EXPECT_TRUE(S_ISREG(st.mode)); }); } diff --git a/src/fspp/fstest/FsppOpenFileTest_Timestamps.h b/src/fspp/fstest/FsppOpenFileTest_Timestamps.h index a5df28ec..f83e9878 100644 --- a/src/fspp/fstest/FsppOpenFileTest_Timestamps.h +++ b/src/fspp/fstest/FsppOpenFileTest_Timestamps.h @@ -14,8 +14,8 @@ public: auto file = this->CreateFile(path); file->truncate(size); auto openFile = file->open(O_RDWR); - assert(this->stat(*openFile).st_size == size); - assert(this->stat(*this->Load(path)).st_size == size); + assert(this->stat(*openFile).size == size); + assert(this->stat(*this->Load(path)).size == size); return openFile; } }; @@ -24,8 +24,7 @@ TYPED_TEST_CASE_P(FsppOpenFileTest_Timestamps); TYPED_TEST_P(FsppOpenFileTest_Timestamps, stat) { auto openFile = this->CreateAndOpenFile("/mynode"); auto operation = [&openFile] () { - struct ::stat st{}; - openFile->stat(&st); + openFile->stat(); }; this->EXPECT_OPERATION_UPDATES_TIMESTAMPS_AS(*openFile, operation, {this->ExpectDoesntUpdateAnyTimestamps}); } diff --git a/src/fspp/fstest/testutils/FileSystemTest.h b/src/fspp/fstest/testutils/FileSystemTest.h index 27cc218a..864f4120 100644 --- a/src/fspp/fstest/testutils/FileSystemTest.h +++ b/src/fspp/fstest/testutils/FileSystemTest.h @@ -90,12 +90,11 @@ public: void setModificationTimestampLaterThanAccessTimestamp(const boost::filesystem::path& path) { auto node = device->Load(path).value(); - struct stat st{}; - node->stat(&st); - st.st_mtim.tv_nsec = st.st_mtim.tv_nsec + 1; + auto st = node->stat(); + st.mtime.tv_nsec = st.mtime.tv_nsec + 1; node->utimens( - st.st_atim, - st.st_mtim + st.atime, + st.mtime ); } }; diff --git a/src/fspp/fstest/testutils/FileTest.h b/src/fspp/fstest/testutils/FileTest.h index 95ae2d14..317124f6 100644 --- a/src/fspp/fstest/testutils/FileTest.h +++ b/src/fspp/fstest/testutils/FileTest.h @@ -28,17 +28,16 @@ public: std::unique_ptr file_nested_node; //TODO IN_STAT still needed after moving it to FsppNodeTest? - void IN_STAT(fspp::File *file, fspp::Node *node, std::function callback) { - struct stat st1{}, st2{}; - node->stat(&st1); + void IN_STAT(fspp::File *file, fspp::Node *node, std::function callback) { + auto st1 = node->stat(); callback(st1); - file->open(O_RDONLY)->stat(&st2); + auto st2 = file->open(O_RDONLY)->stat(); callback(st2); } void EXPECT_SIZE(uint64_t expectedSize, fspp::File *file, fspp::Node *node) { - IN_STAT(file, node, [expectedSize] (struct stat st) { - EXPECT_EQ(expectedSize, static_cast(st.st_size)); + IN_STAT(file, node, [expectedSize] (const fspp::Node::stat_info& st) { + EXPECT_EQ(expectedSize, static_cast(st.size)); }); EXPECT_NUMBYTES_READABLE(expectedSize, file); @@ -53,14 +52,14 @@ public: EXPECT_EQ(expectedSize, static_cast(readBytes)); } - void EXPECT_ATIME_EQ(struct timespec expected, struct stat st) { - EXPECT_EQ(expected.tv_sec, st.st_atim.tv_sec); - EXPECT_EQ(expected.tv_nsec, st.st_atim.tv_nsec); + void EXPECT_ATIME_EQ(struct timespec expected, const fspp::Node::stat_info& st) { + EXPECT_EQ(expected.tv_sec, st.atime.tv_sec); + EXPECT_EQ(expected.tv_nsec, st.atime.tv_nsec); } - void EXPECT_MTIME_EQ(struct timespec expected, struct stat st) { - EXPECT_EQ(expected.tv_sec, st.st_mtim.tv_sec); - EXPECT_EQ(expected.tv_nsec, st.st_mtim.tv_nsec); + void EXPECT_MTIME_EQ(struct timespec expected, const fspp::Node::stat_info& st) { + EXPECT_EQ(expected.tv_sec, st.mtime.tv_sec); + EXPECT_EQ(expected.tv_nsec, st.mtime.tv_nsec); } }; diff --git a/src/fspp/fstest/testutils/FsppNodeTest.h b/src/fspp/fstest/testutils/FsppNodeTest.h index c227a592..a4eaff50 100644 --- a/src/fspp/fstest/testutils/FsppNodeTest.h +++ b/src/fspp/fstest/testutils/FsppNodeTest.h @@ -9,15 +9,14 @@ class FsppNodeTestHelper { public: - void IN_STAT(fspp::Node *file, std::function callback) { - struct stat st{}; - file->stat(&st); + void IN_STAT(fspp::Node *file, std::function callback) { + auto st = file->stat(); callback(st); } void EXPECT_SIZE(uint64_t expectedSize, fspp::Node *node) { - IN_STAT(node, [expectedSize] (struct stat st) { - EXPECT_EQ(expectedSize, static_cast(st.st_size)); + IN_STAT(node, [expectedSize] (const fspp::Node::stat_info& st) { + EXPECT_EQ(expectedSize, static_cast(st.size)); }); } }; diff --git a/src/fspp/fstest/testutils/TimestampTestUtils.h b/src/fspp/fstest/testutils/TimestampTestUtils.h index 362f4a86..42751726 100644 --- a/src/fspp/fstest/testutils/TimestampTestUtils.h +++ b/src/fspp/fstest/testutils/TimestampTestUtils.h @@ -10,7 +10,7 @@ template class TimestampTestUtils : public virtual FileSystemTest { public: - using TimestampUpdateBehavior = std::function; + using TimestampUpdateBehavior = std::function; static TimestampUpdateBehavior ExpectUpdatesAccessTimestamp; static TimestampUpdateBehavior ExpectDoesntUpdateAccessTimestamp; @@ -20,13 +20,13 @@ public: static TimestampUpdateBehavior ExpectDoesntUpdateMetadataTimestamp; static TimestampUpdateBehavior ExpectDoesntUpdateAnyTimestamps; - void EXPECT_OPERATION_UPDATES_TIMESTAMPS_AS(std::function statOld, std::function statNew, std::function operation, std::initializer_list behaviorChecks) { - struct stat oldStat = statOld(); + void EXPECT_OPERATION_UPDATES_TIMESTAMPS_AS(std::function statOld, std::function statNew, std::function operation, std::initializer_list behaviorChecks) { + auto oldStat = statOld(); ensureNodeTimestampsAreOld(oldStat); timespec timeBeforeOperation = cpputils::time::now(); operation(); timespec timeAfterOperation = cpputils::time::now(); - struct stat newStat = statNew(); + auto newStat = statNew(); for (auto behaviorCheck : behaviorChecks) { behaviorCheck(oldStat, newStat, timeBeforeOperation, timeAfterOperation); } @@ -55,30 +55,26 @@ public: } void EXPECT_ACCESS_TIMESTAMP_BETWEEN(timespec lowerBound, timespec upperBound, const fspp::Node &node) { - EXPECT_LE(lowerBound, stat(node).st_atim); - EXPECT_GE(upperBound, stat(node).st_atim); + EXPECT_LE(lowerBound, stat(node).atime); + EXPECT_GE(upperBound, stat(node).atime); } void EXPECT_MODIFICATION_TIMESTAMP_BETWEEN(timespec lowerBound, timespec upperBound, const fspp::Node &node) { - EXPECT_LE(lowerBound, stat(node).st_mtim); - EXPECT_GE(upperBound, stat(node).st_mtim); + EXPECT_LE(lowerBound, stat(node).mtime); + EXPECT_GE(upperBound, stat(node).mtime); } void EXPECT_METADATACHANGE_TIMESTAMP_BETWEEN(timespec lowerBound, timespec upperBound, const fspp::Node &node) { - EXPECT_LE(lowerBound, stat(node).st_ctim); - EXPECT_GE(upperBound, stat(node).st_ctim); + EXPECT_LE(lowerBound, stat(node).ctime); + EXPECT_GE(upperBound, stat(node).ctime); } - static struct stat stat(const fspp::Node &node) { - struct stat st{}; - node.stat(&st); - return st; + static fspp::Node::stat_info stat(const fspp::Node &node) { + return node.stat(); } - static struct stat stat(const fspp::OpenFile &openFile) { - struct stat st{}; - openFile.stat(&st); - return st; + static fspp::Node::stat_info stat(const fspp::OpenFile &openFile) { + return openFile.stat(); } timespec xSecondsAgo(int sec) { @@ -87,11 +83,11 @@ public: return result; } - void ensureNodeTimestampsAreOld(const struct stat &nodeStat) { + void ensureNodeTimestampsAreOld(const fspp::Node::stat_info &nodeStat) { waitUntilClockProgresses(); - EXPECT_LT(nodeStat.st_atim, cpputils::time::now()); - EXPECT_LT(nodeStat.st_mtim, cpputils::time::now()); - EXPECT_LT(nodeStat.st_ctim, cpputils::time::now()); + EXPECT_LT(nodeStat.atime, cpputils::time::now()); + EXPECT_LT(nodeStat.mtime, cpputils::time::now()); + EXPECT_LT(nodeStat.ctime, cpputils::time::now()); } private: @@ -106,57 +102,57 @@ private: template typename TimestampTestUtils::TimestampUpdateBehavior TimestampTestUtils::ExpectUpdatesAccessTimestamp = - [] (struct stat statBeforeOperation, struct stat statAfterOperation, timespec timeBeforeOperation, timespec timeAfterOperation) { + [] (const fspp::Node::stat_info& statBeforeOperation, const fspp::Node::stat_info& statAfterOperation, timespec timeBeforeOperation, timespec timeAfterOperation) { UNUSED(statBeforeOperation); UNUSED(timeBeforeOperation); UNUSED(timeAfterOperation); - EXPECT_LE(timeBeforeOperation, statAfterOperation.st_atim); - EXPECT_GE(timeAfterOperation, statAfterOperation.st_atim); + EXPECT_LE(timeBeforeOperation, statAfterOperation.atime); + EXPECT_GE(timeAfterOperation, statAfterOperation.atime); }; template typename TimestampTestUtils::TimestampUpdateBehavior TimestampTestUtils::ExpectDoesntUpdateAccessTimestamp = - [] (struct stat statBeforeOperation, struct stat statAfterOperation, timespec timeBeforeOperation, timespec timeAfterOperation) { + [] (const fspp::Node::stat_info& statBeforeOperation, const fspp::Node::stat_info& statAfterOperation, timespec timeBeforeOperation, timespec timeAfterOperation) { UNUSED(timeBeforeOperation); UNUSED(timeAfterOperation); - EXPECT_EQ(statBeforeOperation.st_atim, statAfterOperation.st_atim); + EXPECT_EQ(statBeforeOperation.atime, statAfterOperation.atime); }; template typename TimestampTestUtils::TimestampUpdateBehavior TimestampTestUtils::ExpectUpdatesModificationTimestamp = - [] (struct stat statBeforeOperation, struct stat statAfterOperation, timespec timeBeforeOperation, timespec timeAfterOperation) { + [] (const fspp::Node::stat_info& statBeforeOperation, const fspp::Node::stat_info& statAfterOperation, timespec timeBeforeOperation, timespec timeAfterOperation) { UNUSED(statBeforeOperation); - EXPECT_LE(timeBeforeOperation, statAfterOperation.st_mtim); - EXPECT_GE(timeAfterOperation, statAfterOperation.st_mtim); + EXPECT_LE(timeBeforeOperation, statAfterOperation.mtime); + EXPECT_GE(timeAfterOperation, statAfterOperation.mtime); }; template typename TimestampTestUtils::TimestampUpdateBehavior TimestampTestUtils::ExpectDoesntUpdateModificationTimestamp = - [] (struct stat statBeforeOperation, struct stat statAfterOperation, timespec timeBeforeOperation, timespec timeAfterOperation) { + [] (const fspp::Node::stat_info& statBeforeOperation, const fspp::Node::stat_info& statAfterOperation, timespec timeBeforeOperation, timespec timeAfterOperation) { UNUSED(timeBeforeOperation); UNUSED(timeAfterOperation); - EXPECT_EQ(statBeforeOperation.st_mtim, statAfterOperation.st_mtim); + EXPECT_EQ(statBeforeOperation.mtime, statAfterOperation.mtime); }; template typename TimestampTestUtils::TimestampUpdateBehavior TimestampTestUtils::ExpectUpdatesMetadataTimestamp = - [] (struct stat statBeforeOperation, struct stat statAfterOperation, timespec timeBeforeOperation, timespec timeAfterOperation) { + [] (const fspp::Node::stat_info& statBeforeOperation, const fspp::Node::stat_info& statAfterOperation, timespec timeBeforeOperation, timespec timeAfterOperation) { UNUSED(statBeforeOperation); - EXPECT_LE(timeBeforeOperation, statAfterOperation.st_ctim); - EXPECT_GE(timeAfterOperation, statAfterOperation.st_ctim); + EXPECT_LE(timeBeforeOperation, statAfterOperation.ctime); + EXPECT_GE(timeAfterOperation, statAfterOperation.ctime); }; template typename TimestampTestUtils::TimestampUpdateBehavior TimestampTestUtils::ExpectDoesntUpdateMetadataTimestamp = - [] (struct stat statBeforeOperation, struct stat statAfterOperation, timespec timeBeforeOperation, timespec timeAfterOperation) { + [] (const fspp::Node::stat_info& statBeforeOperation, const fspp::Node::stat_info& statAfterOperation, timespec timeBeforeOperation, timespec timeAfterOperation) { UNUSED(timeBeforeOperation); UNUSED(timeAfterOperation); - EXPECT_EQ(statBeforeOperation.st_ctim, statAfterOperation.st_ctim); + EXPECT_EQ(statBeforeOperation.ctime, statAfterOperation.ctime); }; template typename TimestampTestUtils::TimestampUpdateBehavior TimestampTestUtils::ExpectDoesntUpdateAnyTimestamps = - [] (struct stat statBeforeOperation, struct stat statAfterOperation, timespec timeBeforeOperation, timespec timeAfterOperation) { + [] (const fspp::Node::stat_info& statBeforeOperation, const fspp::Node::stat_info& statAfterOperation, timespec timeBeforeOperation, timespec timeAfterOperation) { ExpectDoesntUpdateAccessTimestamp(statBeforeOperation, statAfterOperation, timeBeforeOperation, timeAfterOperation); ExpectDoesntUpdateModificationTimestamp(statBeforeOperation, statAfterOperation, timeBeforeOperation, timeAfterOperation); ExpectDoesntUpdateMetadataTimestamp(statBeforeOperation, statAfterOperation, timeBeforeOperation, timeAfterOperation); diff --git a/src/fspp/impl/FilesystemImpl.cpp b/src/fspp/impl/FilesystemImpl.cpp index f72d0df9..9ff74775 100644 --- a/src/fspp/impl/FilesystemImpl.cpp +++ b/src/fspp/impl/FilesystemImpl.cpp @@ -137,19 +137,35 @@ void FilesystemImpl::closeFile(int descriptor) { _open_files.close(descriptor); } +namespace { +void convert_stat_info(const fspp::Node::stat_info& input, struct ::stat *output) { + output->st_nlink = input.nlink; + output->st_mode = input.mode; + output->st_uid = input.uid; + output->st_gid = input.gid; + output->st_size = input.size; + output->st_blocks = input.blocks; + output->st_atim = input.atime; + output->st_mtim = input.mtime; + output->st_ctim = input.ctime; +} +} + void FilesystemImpl::lstat(const bf::path &path, struct ::stat *stbuf) { PROFILE(_lstatNanosec); auto node = _device->Load(path); if(node == none) { throw fuse::FuseErrnoException(ENOENT); } else { - (*node)->stat(stbuf); + auto stat_info = (*node)->stat(); + convert_stat_info(stat_info, stbuf); } } void FilesystemImpl::fstat(int descriptor, struct ::stat *stbuf) { PROFILE(_fstatNanosec); - _open_files.get(descriptor)->stat(stbuf); + auto stat_info = _open_files.get(descriptor)->stat(); + convert_stat_info(stat_info, stbuf); } void FilesystemImpl::chmod(const boost::filesystem::path &path, mode_t mode) { diff --git a/test/fspp/impl/FuseOpenFileListTest.cpp b/test/fspp/impl/FuseOpenFileListTest.cpp index be104fd8..a2cc67ae 100644 --- a/test/fspp/impl/FuseOpenFileListTest.cpp +++ b/test/fspp/impl/FuseOpenFileListTest.cpp @@ -17,7 +17,7 @@ public: ~MockOpenFile() {destructed = true;} - MOCK_CONST_METHOD1(stat, void(struct ::stat*)); + MOCK_CONST_METHOD0(stat, OpenFile::stat_info()); MOCK_CONST_METHOD1(truncate, void(off_t)); MOCK_CONST_METHOD3(read, size_t(void*, size_t, off_t)); MOCK_METHOD3(write, void(const void*, size_t, off_t)); From 4430ca11e9bc9ea68d397424b5bd7e9f40909aed Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Thu, 13 Sep 2018 21:36:50 -0700 Subject: [PATCH 2/3] Handle struct ::stat correctly, whether it has st_atim or st_atime members --- src/fspp/impl/FilesystemImpl.cpp | 37 ++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/src/fspp/impl/FilesystemImpl.cpp b/src/fspp/impl/FilesystemImpl.cpp index 9ff74775..a16ab654 100644 --- a/src/fspp/impl/FilesystemImpl.cpp +++ b/src/fspp/impl/FilesystemImpl.cpp @@ -138,16 +138,41 @@ void FilesystemImpl::closeFile(int descriptor) { } namespace { -void convert_stat_info(const fspp::Node::stat_info& input, struct ::stat *output) { +// Implementation taken from http://en.cppreference.com/w/cpp/types/void_t +// (it takes CWG1558 into account and also works for older compilers) +template struct make_void { typedef void type;}; +template using void_t = typename make_void::type; + +template struct uses_timespec final : std::false_type {}; +template struct uses_timespec> final : std::true_type {}; + +// convert_stat_info_timestamps_ looks if struct ::stat has st_atim or st_atime members +// and sets the correct ones. +template struct convert_stat_info_timestamps_ final {}; +template struct convert_stat_info_timestamps_::value>> final { + static void call(const fspp::Node::stat_info& input, Stat* output) { + output->st_atim = input.atime; + output->st_mtim = input.mtime; + output->st_ctim = input.ctime; + } +}; +template struct convert_stat_info_timestamps_::value>> final { + static void call(const fspp::Node::stat_info& input, Stat* output) { + output->st_atime = input.atime.tv_sec; + output->st_mtime = input.mtime.tv_sec; + output->st_ctime = input.ctime.tv_sec; + } +}; + + +void convert_stat_info_(const fspp::Node::stat_info& input, struct ::stat *output) { output->st_nlink = input.nlink; output->st_mode = input.mode; output->st_uid = input.uid; output->st_gid = input.gid; output->st_size = input.size; output->st_blocks = input.blocks; - output->st_atim = input.atime; - output->st_mtim = input.mtime; - output->st_ctim = input.ctime; + convert_stat_info_timestamps_::call(input, output); } } @@ -158,14 +183,14 @@ void FilesystemImpl::lstat(const bf::path &path, struct ::stat *stbuf) { throw fuse::FuseErrnoException(ENOENT); } else { auto stat_info = (*node)->stat(); - convert_stat_info(stat_info, stbuf); + convert_stat_info_(stat_info, stbuf); } } void FilesystemImpl::fstat(int descriptor, struct ::stat *stbuf) { PROFILE(_fstatNanosec); auto stat_info = _open_files.get(descriptor)->stat(); - convert_stat_info(stat_info, stbuf); + convert_stat_info_(stat_info, stbuf); } void FilesystemImpl::chmod(const boost::filesystem::path &path, mode_t mode) { From cb852c16ccda6313fa07ce991b95fd18510ba84a Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Thu, 13 Sep 2018 21:42:54 -0700 Subject: [PATCH 3/3] remove metaprogramming --- src/fspp/fstest/FsppFileTest_Timestamps.h | 2 +- src/fspp/impl/FilesystemImpl.cpp | 32 +++-------------------- 2 files changed, 5 insertions(+), 29 deletions(-) diff --git a/src/fspp/fstest/FsppFileTest_Timestamps.h b/src/fspp/fstest/FsppFileTest_Timestamps.h index 9399d944..67ba5eab 100644 --- a/src/fspp/fstest/FsppFileTest_Timestamps.h +++ b/src/fspp/fstest/FsppFileTest_Timestamps.h @@ -10,7 +10,7 @@ public: cpputils::unique_ref CreateFileWithSize(const boost::filesystem::path &path, off_t size) { auto file = this->CreateFile(path); file->truncate(size); - assert(this->stat(*this->Load(path)).st_size == size); + assert(this->stat(*this->Load(path)).size == size); return file; } }; diff --git a/src/fspp/impl/FilesystemImpl.cpp b/src/fspp/impl/FilesystemImpl.cpp index a16ab654..2228aa3d 100644 --- a/src/fspp/impl/FilesystemImpl.cpp +++ b/src/fspp/impl/FilesystemImpl.cpp @@ -11,6 +11,7 @@ #include #include +#include #include using namespace fspp; @@ -138,33 +139,6 @@ void FilesystemImpl::closeFile(int descriptor) { } namespace { -// Implementation taken from http://en.cppreference.com/w/cpp/types/void_t -// (it takes CWG1558 into account and also works for older compilers) -template struct make_void { typedef void type;}; -template using void_t = typename make_void::type; - -template struct uses_timespec final : std::false_type {}; -template struct uses_timespec> final : std::true_type {}; - -// convert_stat_info_timestamps_ looks if struct ::stat has st_atim or st_atime members -// and sets the correct ones. -template struct convert_stat_info_timestamps_ final {}; -template struct convert_stat_info_timestamps_::value>> final { - static void call(const fspp::Node::stat_info& input, Stat* output) { - output->st_atim = input.atime; - output->st_mtim = input.mtime; - output->st_ctim = input.ctime; - } -}; -template struct convert_stat_info_timestamps_::value>> final { - static void call(const fspp::Node::stat_info& input, Stat* output) { - output->st_atime = input.atime.tv_sec; - output->st_mtime = input.mtime.tv_sec; - output->st_ctime = input.ctime.tv_sec; - } -}; - - void convert_stat_info_(const fspp::Node::stat_info& input, struct ::stat *output) { output->st_nlink = input.nlink; output->st_mode = input.mode; @@ -172,7 +146,9 @@ void convert_stat_info_(const fspp::Node::stat_info& input, struct ::stat *outpu output->st_gid = input.gid; output->st_size = input.size; output->st_blocks = input.blocks; - convert_stat_info_timestamps_::call(input, output); + output->st_atim = input.atime; + output->st_mtim = input.mtime; + output->st_ctim = input.ctime; } }