Refactoring: Factor out a DirEntryList class from DirBlob

src/filesystem/cachingfsblobstore/DirBlobRef.h

@@ -15,7 +15,7 @@ public:
         ASSERT(_base != nullptr, "We just initialized this with a pointer to DirBlob. Can't be something else now.");
     }
 
-    using Entry = fsblobstore::DirBlob::Entry;
+    using Entry = fsblobstore::DirEntry;
 
     const Entry &GetChild(const std::string &name) const {
         return _base->GetChild(name);

src/filesystem/fsblobstore/DirBlob.cpp

@@ -26,8 +26,6 @@ using boost::none;
 namespace cryfs {
 namespace fsblobstore {
 
-//TODO Factor out a DirEntryList class
-
 DirBlob::DirBlob(unique_ref<Blob> blob, std::function<off_t (const blockstore::Key&)> getLstatSize) :
     FsBlob(std::move(blob)), _getLstatSize(getLstatSize), _entries(), _changed(false) {
   ASSERT(magicNumber() == MagicNumbers::DIR, "Loaded blob is not a directory");
@@ -50,94 +48,19 @@ unique_ref<DirBlob> DirBlob::InitializeEmptyDir(unique_ref<Blob> blob, std::func
   return make_unique_ref<DirBlob>(std::move(blob), getLstatSize);
 }
 
-size_t DirBlob::_serializedSizeOfEntry(const DirBlob::Entry &entry) {
-  return 1 + (entry.name.size() + 1) + entry.key.BINARY_LENGTH + sizeof(uid_t) + sizeof(gid_t) + sizeof(mode_t);
-}
-
-void DirBlob::_serializeEntry(const DirBlob::Entry & entry, uint8_t *dest) {
-  unsigned int offset = 0;
-  *(dest+offset) = static_cast<uint8_t>(entry.type);
-  offset += 1;
-
-  std::memcpy(dest+offset, entry.name.c_str(), entry.name.size()+1);
-  offset += entry.name.size() + 1;
-
-  entry.key.ToBinary(dest+offset);
-  offset += entry.key.BINARY_LENGTH;
-
-  *reinterpret_cast<uid_t*>(dest+offset) = entry.uid;
-  offset += sizeof(uid_t);
-
-  *reinterpret_cast<gid_t*>(dest+offset) = entry.gid;
-  offset += sizeof(gid_t);
-
-  *reinterpret_cast<mode_t*>(dest+offset) = entry.mode;
-  offset += sizeof(mode_t);
-
-  ASSERT(offset == _serializedSizeOfEntry(entry), "Didn't write correct number of elements");
-}
-
 void DirBlob::_writeEntriesToBlob() {
   if (_changed) {
-    size_t serializedSize = 0;
+    Data serialized = _entries.serialize();
-    for (const auto &entry : _entries) {
+    baseBlob().resize(1 + serialized.size());
-      serializedSize += _serializedSizeOfEntry(entry);
+    baseBlob().write(serialized.data(), 1, serialized.size());
-    }
-    Data serialized(serializedSize);
-    unsigned int offset = 0;
-    for (const auto &entry : _entries) {
-      _serializeEntry(entry, static_cast<uint8_t*>(serialized.dataOffset(offset)));
-      offset += _serializedSizeOfEntry(entry);
-    }
-    baseBlob().resize(1 + serializedSize);
-    baseBlob().write(serialized.data(), 1, serializedSize);
     _changed = false;
   }
 }
 
 void DirBlob::_readEntriesFromBlob() {
   //No lock needed, because this is only called from the constructor.
-  _entries.clear();
   Data data = baseBlob().readAll();
-
+  _entries.deserializeFrom(static_cast<uint8_t*>(data.data()) + 1, data.size() - 1);  // data+1/size-1 because the first byte is the magic number
-  const char *pos = (const char*)data.data() + 1; // +1 for magic number of blob
-  while (pos < (const char*) data.data() + data.size()) {
-    pos = readAndAddNextChild(pos, &_entries);
-  }
-}
-
-const char *DirBlob::readAndAddNextChild(const char *pos,
-    vector<DirBlob::Entry> *result) const {
-  // Read type magic number (whether it is a dir or a file)
-  fspp::Dir::EntryType type =
-      static_cast<fspp::Dir::EntryType>(*reinterpret_cast<const unsigned char*>(pos));
-  pos += 1;
-
-  size_t namelength = strlen(pos);
-  std::string name(pos, namelength);
-  pos += namelength + 1;
-
-  Key key = Key::FromBinary(pos);
-  pos += Key::BINARY_LENGTH;
-
-  uid_t uid = *(uid_t*)pos;
-  pos += sizeof(uid_t);
-
-  gid_t gid = *(gid_t*)pos;
-  pos += sizeof(gid_t);
-
-  mode_t mode = *(mode_t*)pos;
-  pos += sizeof(mode_t);
-
-  result->emplace_back(type, name, key, mode, uid, gid);
-  return pos;
-}
-
-bool DirBlob::_hasChild(const string &name) const {
-  auto found = std::find_if(_entries.begin(), _entries.end(), [name] (const Entry &entry) {
-    return entry.name == name;
-  });
-  return found != _entries.end();
 }
 
 void DirBlob::AddChildDir(const std::string &name, const Key &blobKey, mode_t mode, uid_t uid, gid_t gid) {
@@ -155,54 +78,26 @@ void DirBlob::AddChildSymlink(const std::string &name, const blockstore::Key &bl
 void DirBlob::AddChild(const std::string &name, const Key &blobKey,
     fspp::Dir::EntryType entryType, mode_t mode, uid_t uid, gid_t gid) {
   std::unique_lock<std::mutex> lock(_mutex);
-  if (_hasChild(name)) {
+  _entries.add(name, blobKey, entryType, mode, uid, gid);
-    throw fspp::fuse::FuseErrnoException(EEXIST);
-  }
-
-  _entries.emplace_back(entryType, name, blobKey, mode, uid, gid);
   _changed = true;
 }
 
-const DirBlob::Entry &DirBlob::GetChild(const string &name) const {
+const DirEntry &DirBlob::GetChild(const string &name) const {
   std::unique_lock<std::mutex> lock(_mutex);
-  auto found = std::find_if(_entries.begin(), _entries.end(), [name] (const Entry &entry) {
+  return _entries.get(name);
-    return entry.name == name;
-  });
-  if (found == _entries.end()) {
-    throw fspp::fuse::FuseErrnoException(ENOENT);
-  }
-  return *found;
 }
 
-const DirBlob::Entry &DirBlob::GetChild(const Key &key) const {
+const DirEntry &DirBlob::GetChild(const Key &key) const {
   std::unique_lock<std::mutex> lock(_mutex);
-  auto found = std::find_if(_entries.begin(), _entries.end(), [key] (const Entry &entry) {
+  return _entries.get(key);
-	return entry.key == key;
-  });
-  if (found == _entries.end()) {
-	throw fspp::fuse::FuseErrnoException(ENOENT);
-  }
-  return *found;
 }
 
 void DirBlob::RemoveChild(const Key &key) {
   std::unique_lock<std::mutex> lock(_mutex);
-  auto found = _findChild(key);
+  _entries.remove(key);
-  _entries.erase(found);
   _changed = true;
 }
 
-std::vector<DirBlob::Entry>::iterator DirBlob::_findChild(const Key &key) {
-  //TODO Code duplication with GetChild(key)
-  auto found = std::find_if(_entries.begin(), _entries.end(), [key] (const Entry &entry) {
-	return entry.key == key;
-  });
-  if (found == _entries.end()) {
-	throw fspp::fuse::FuseErrnoException(ENOENT);
-  }
-  return found;
-}
-
 void DirBlob::AppendChildrenTo(vector<fspp::Dir::Entry> *result) const {
   std::unique_lock<std::mutex> lock(_mutex);
   result->reserve(result->size() + _entries.size());
@@ -235,21 +130,13 @@ void DirBlob::statChild(const Key &key, struct ::stat *result) const {
 
 void DirBlob::chmodChild(const Key &key, mode_t mode) {
   std::unique_lock<std::mutex> lock(_mutex);
-  auto found = _findChild(key);
+  _entries.setMode(key, mode);
-  ASSERT ((S_ISREG(mode) && S_ISREG(found->mode)) || (S_ISDIR(mode) && S_ISDIR(found->mode)) || (S_ISLNK(mode)), "Unknown mode in entry");
-  found->mode = mode;
   _changed = true;
 }
 
 void DirBlob::chownChild(const Key &key, uid_t uid, gid_t gid) {
   std::unique_lock<std::mutex> lock(_mutex);
-  auto found = _findChild(key);
+  if(_entries.setUidGid(key, uid, gid)) {
-  if (uid != (uid_t)-1) {
-    found->uid = uid;
-    _changed = true;
-  }
-  if (gid != (gid_t)-1) {
-    found->gid = gid;
     _changed = true;
   }
 }

src/filesystem/fsblobstore/DirBlob.h

@@ -6,7 +6,7 @@
 #include <messmer/cpp-utils/macros.h>
 #include <messmer/fspp/fs_interface/Dir.h>
 #include "FsBlob.h"
-#include <vector>
+#include "utils/DirEntryList.h"
 #include <mutex>
 
 namespace cryfs {
@@ -15,32 +15,6 @@ namespace cryfs {
 
         class DirBlob : public FsBlob {
         public:
-            struct Entry {
-                Entry(fspp::Dir::EntryType type_, const std::string &name_, const blockstore::Key &key_, mode_t mode_,
-                      uid_t uid_, gid_t gid_) : type(type_), name(name_), key(key_), mode(mode_), uid(uid_), gid(gid_) {
-                  switch (type) {
-                    case fspp::Dir::EntryType::FILE:
-                      mode |= S_IFREG;
-                          break;
-                    case fspp::Dir::EntryType::DIR:
-                      mode |= S_IFDIR;
-                          break;
-                    case fspp::Dir::EntryType::SYMLINK:
-                      mode |= S_IFLNK;
-                          break;
-                  }
-                  ASSERT((S_ISREG(mode) && type == fspp::Dir::EntryType::FILE) ||
-                         (S_ISDIR(mode) && type == fspp::Dir::EntryType::DIR) ||
-                         (S_ISLNK(mode) && type == fspp::Dir::EntryType::SYMLINK), "Unknown mode in entry");
-                }
-
-                fspp::Dir::EntryType type;
-                std::string name;
-                blockstore::Key key;
-                mode_t mode;
-                uid_t uid;
-                gid_t gid;
-            };
 
             static cpputils::unique_ref<DirBlob> InitializeEmptyDir(cpputils::unique_ref<blobstore::Blob> blob,
                                                                     std::function<off_t (const blockstore::Key&)> getLstatSize);
@@ -53,9 +27,9 @@ namespace cryfs {
 
             void AppendChildrenTo(std::vector<fspp::Dir::Entry> *result) const;
 
-            const Entry &GetChild(const std::string &name) const;
+            const DirEntry &GetChild(const std::string &name) const;
 
-            const Entry &GetChild(const blockstore::Key &key) const;
+            const DirEntry &GetChild(const blockstore::Key &key) const;
 
             void AddChildDir(const std::string &name, const blockstore::Key &blobKey, mode_t mode, uid_t uid,
                              gid_t gid);
@@ -83,22 +57,13 @@ namespace cryfs {
 
         private:
 
-            const char *readAndAddNextChild(const char *pos, std::vector<Entry> *result) const;
-
-            bool _hasChild(const std::string &name) const;
-
             void _readEntriesFromBlob();
-
             void _writeEntriesToBlob();
-            static size_t _serializedSizeOfEntry(const DirBlob::Entry &entry);
-            static void _serializeEntry(const DirBlob::Entry &entry, uint8_t *dest);
 
             cpputils::unique_ref<blobstore::Blob> releaseBaseBlob() override;
 
-            std::vector<DirBlob::Entry>::iterator _findChild(const blockstore::Key &key);
-
             std::function<off_t (const blockstore::Key&)> _getLstatSize;
-            std::vector<Entry> _entries;
+            DirEntryList _entries;
             mutable std::mutex _mutex;
             bool _changed;
 

src/filesystem/fsblobstore/utils/DirEntry.cpp

@@ -0,0 +1,63 @@
+#include "DirEntry.h"
+
+using std::vector;
+using blockstore::Key;
+
+namespace cryfs {
+    namespace fsblobstore {
+
+        void DirEntry::serialize(uint8_t *dest) const {
+            unsigned int offset = 0;
+            *(dest+offset) = static_cast<uint8_t>(type);
+            offset += 1;
+
+            std::memcpy(dest+offset, name.c_str(), name.size()+1);
+            offset += name.size() + 1;
+
+            key.ToBinary(dest+offset);
+            offset += key.BINARY_LENGTH;
+
+            *reinterpret_cast<uid_t*>(dest+offset) = uid;
+            offset += sizeof(uid_t);
+
+            *reinterpret_cast<gid_t*>(dest+offset) = gid;
+            offset += sizeof(gid_t);
+
+            *reinterpret_cast<mode_t*>(dest+offset) = mode;
+            offset += sizeof(mode_t);
+
+            ASSERT(offset == serializedSize(), "Didn't write correct number of elements");
+        }
+
+        size_t DirEntry::serializedSize() const {
+            return 1 + (name.size() + 1) + key.BINARY_LENGTH + sizeof(uid_t) + sizeof(gid_t) + sizeof(mode_t);
+        }
+
+        const char *DirEntry::deserializeAndAddToVector(const char *pos, vector<DirEntry> *result) {
+            // Read type magic number (whether it is a dir or a file)
+            fspp::Dir::EntryType type =
+                    static_cast<fspp::Dir::EntryType>(*reinterpret_cast<const unsigned char*>(pos));
+            pos += 1;
+
+            size_t namelength = strlen(pos);
+            std::string name(pos, namelength);
+            pos += namelength + 1;
+
+            Key key = Key::FromBinary(pos);
+            pos += Key::BINARY_LENGTH;
+
+            uid_t uid = *(uid_t*)pos;
+            pos += sizeof(uid_t);
+
+            gid_t gid = *(gid_t*)pos;
+            pos += sizeof(gid_t);
+
+            mode_t mode = *(mode_t*)pos;
+            pos += sizeof(mode_t);
+
+            result->emplace_back(type, name, key, mode, uid, gid);
+            return pos;
+        }
+
+    }
+}

src/filesystem/fsblobstore/utils/DirEntry.h

@@ -0,0 +1,45 @@
+#pragma once
+#ifndef CRYFS_FILESYSTEM_FSBLOBSTORE_UTILS_DIRENTRY_H
+#define CRYFS_FILESYSTEM_FSBLOBSTORE_UTILS_DIRENTRY_H
+
+#include <messmer/blockstore/utils/Key.h>
+#include <messmer/fspp/fs_interface/Dir.h>
+
+namespace cryfs {
+    namespace fsblobstore {
+
+        struct DirEntry {
+            DirEntry(fspp::Dir::EntryType type_, const std::string &name_, const blockstore::Key &key_, mode_t mode_,
+                  uid_t uid_, gid_t gid_) : type(type_), name(name_), key(key_), mode(mode_), uid(uid_), gid(gid_) {
+                switch (type) {
+                    case fspp::Dir::EntryType::FILE:
+                        mode |= S_IFREG;
+                        break;
+                    case fspp::Dir::EntryType::DIR:
+                        mode |= S_IFDIR;
+                        break;
+                    case fspp::Dir::EntryType::SYMLINK:
+                        mode |= S_IFLNK;
+                        break;
+                }
+                ASSERT((S_ISREG(mode) && type == fspp::Dir::EntryType::FILE) ||
+                       (S_ISDIR(mode) && type == fspp::Dir::EntryType::DIR) ||
+                       (S_ISLNK(mode) && type == fspp::Dir::EntryType::SYMLINK), "Unknown mode in entry");
+            }
+
+            void serialize(uint8_t* dest) const;
+            size_t serializedSize() const;
+            static const char *deserializeAndAddToVector(const char *pos, std::vector<DirEntry> *result);
+
+            fspp::Dir::EntryType type;
+            std::string name;
+            blockstore::Key key;
+            mode_t mode;
+            uid_t uid;
+            gid_t gid;
+        };
+
+    }
+}
+
+#endif

src/filesystem/fsblobstore/utils/DirEntryList.cpp

@@ -0,0 +1,122 @@
+#include "DirEntryList.h"
+
+//TODO Get rid of that in favor of better error handling
+#include <messmer/fspp/fuse/FuseErrnoException.h>
+
+using cpputils::Data;
+using std::string;
+using std::vector;
+using blockstore::Key;
+
+namespace cryfs {
+namespace fsblobstore {
+
+Data DirEntryList::serialize() const {
+    Data serialized(_serializedSize());
+    unsigned int offset = 0;
+    for (const auto &entry : _entries) {
+        entry.serialize(static_cast<uint8_t*>(serialized.dataOffset(offset)));
+        offset += entry.serializedSize();
+    }
+    return serialized;
+}
+
+uint64_t DirEntryList::_serializedSize() const {
+    uint64_t serializedSize = 0;
+    for (const auto &entry : _entries) {
+        serializedSize += entry.serializedSize();
+    }
+    return serializedSize;
+}
+
+void DirEntryList::deserializeFrom(const void *data, uint64_t size) {
+    _entries.clear();
+    const char *pos = static_cast<const char*>(data);
+    while (pos < static_cast<const char*>(data) + size) {
+        pos = DirEntry::deserializeAndAddToVector(pos, &_entries);
+    }
+}
+
+bool DirEntryList::_hasChild(const string &name) const {
+    auto found = std::find_if(_entries.begin(), _entries.end(), [name] (const DirEntry &entry) {
+        return entry.name == name;
+    });
+    return found != _entries.end();
+}
+
+void DirEntryList::add(const string &name, const Key &blobKey, fspp::Dir::EntryType entryType, mode_t mode,
+                            uid_t uid, gid_t gid) {
+    if (_hasChild(name)) {
+        throw fspp::fuse::FuseErrnoException(EEXIST);
+    }
+
+    _entries.emplace_back(entryType, name, blobKey, mode, uid, gid);
+}
+
+const DirEntry &DirEntryList::get(const string &name) const {
+    auto found = std::find_if(_entries.begin(), _entries.end(), [name] (const DirEntry &entry) {
+        return entry.name == name;
+    });
+    if (found == _entries.end()) {
+        throw fspp::fuse::FuseErrnoException(ENOENT);
+    }
+    return *found;
+}
+
+const DirEntry &DirEntryList::get(const Key &key) const {
+    return *_find(key);
+}
+
+void DirEntryList::remove(const Key &key) {
+    auto found = _find(key);
+    _entries.erase(found);
+}
+
+vector<DirEntry>::iterator DirEntryList::_find(const Key &key) {
+    auto found = std::find_if(_entries.begin(), _entries.end(), [key] (const DirEntry &entry) {
+        return entry.key == key;
+    });
+    if (found == _entries.end()) {
+        throw fspp::fuse::FuseErrnoException(ENOENT);
+    }
+    return found;
+}
+
+vector<DirEntry>::const_iterator DirEntryList::_find(const Key &key) const {
+    return const_cast<DirEntryList*>(this)->_find(key);
+}
+
+size_t DirEntryList::size() const {
+    return _entries.size();
+}
+
+DirEntryList::const_iterator DirEntryList::begin() const {
+    return _entries.begin();
+}
+
+DirEntryList::const_iterator DirEntryList::end() const {
+    return _entries.end();
+}
+
+void DirEntryList::setMode(const Key &key, mode_t mode) {
+    auto found = _find(key);
+    ASSERT ((S_ISREG(mode) && S_ISREG(found->mode)) || (S_ISDIR(mode) && S_ISDIR(found->mode)) || (S_ISLNK(mode)), "Unknown mode in entry");
+    found->mode = mode;
+}
+
+bool DirEntryList::setUidGid(const Key &key, uid_t uid, gid_t gid) {
+    auto found = _find(key);
+    bool changed = false;
+    if (uid != (uid_t)-1) {
+        found->uid = uid;
+        changed = true;
+    }
+    if (gid != (gid_t)-1) {
+        found->gid = gid;
+        changed = true;
+    }
+    return changed;
+}
+
+}
+}

src/filesystem/fsblobstore/utils/DirEntryList.h

@@ -0,0 +1,45 @@
+#pragma once
+#ifndef CRYFS_FILESYSTEM_FSBLOBSTORE_UTILS_DIRENTRYLIST_H
+#define CRYFS_FILESYSTEM_FSBLOBSTORE_UTILS_DIRENTRYLIST_H
+
+#include <messmer/cpp-utils/data/Data.h>
+#include "DirEntry.h"
+#include <vector>
+#include <string>
+
+namespace cryfs {
+    namespace fsblobstore {
+
+        class DirEntryList {
+        public:
+            using const_iterator = std::vector<DirEntry>::const_iterator;
+
+            cpputils::Data serialize() const;
+            void deserializeFrom(const void *data, uint64_t size);
+
+            void add(const std::string &name, const blockstore::Key &blobKey, fspp::Dir::EntryType entryType,
+                     mode_t mode, uid_t uid, gid_t gid);
+            const DirEntry &get(const std::string &name) const;
+            const DirEntry &get(const blockstore::Key &key) const;
+            void remove(const blockstore::Key &key);
+
+            size_t size() const;
+            const_iterator begin() const;
+            const_iterator end() const;
+
+            void setMode(const blockstore::Key &key, mode_t mode);
+            bool setUidGid(const blockstore::Key &key, uid_t uid, gid_t gid);
+
+        private:
+            uint64_t _serializedSize() const;
+            bool _hasChild(const std::string &name) const;
+            std::vector<DirEntry>::iterator _find(const blockstore::Key &key);
+            std::vector<DirEntry>::const_iterator _find(const blockstore::Key &key) const;
+
+            std::vector<DirEntry> _entries;
+        };
+
+    }
+}
+
+#endif

src/filesystem/parallelaccessfsblobstore/DirBlobRef.h

@@ -11,7 +11,7 @@ class DirBlobRef: public FsBlobRef {
 public:
     DirBlobRef(cachingfsblobstore::DirBlobRef *base): _base(base) {}
 
-    using Entry = fsblobstore::DirBlob::Entry;
+    using Entry = fsblobstore::DirEntry;
 
     const Entry &GetChild(const std::string &name) const {
         return _base->GetChild(name);