From deaa293c67410292c76bf615b6ee4f3e5d02db31 Mon Sep 17 00:00:00 2001
From: Gary Talent <gtalent2@gmail.com>
Date: Sun, 3 Nov 2019 16:44:57 -0600
Subject: [PATCH] [ox/fs] Fix problems with creating and reading directories
---
.../src/ox/fs/filestore/filestoretemplate.hpp | 1 +
deps/ox/src/ox/fs/filesystem/directory.hpp | 44 ++++++++---------
deps/ox/src/ox/fs/filesystem/filesystem.cpp | 2 +-
deps/ox/src/ox/fs/filesystem/filesystem.hpp | 17 +++----
.../ox/src/ox/fs/filesystem/passthroughfs.cpp | 4 +-
.../ox/src/ox/fs/filesystem/passthroughfs.hpp | 4 +-
deps/ox/src/ox/fs/filesystem/pathiterator.cpp | 47 ++++++++++---------
7 files changed, 63 insertions(+), 56 deletions(-)
diff --git a/deps/ox/src/ox/fs/filestore/filestoretemplate.hpp b/deps/ox/src/ox/fs/filestore/filestoretemplate.hpp
index accd02a0..77961779 100644
--- a/deps/ox/src/ox/fs/filestore/filestoretemplate.hpp
+++ b/deps/ox/src/ox/fs/filestore/filestoretemplate.hpp
@@ -729,6 +729,7 @@ typename FileStoreTemplate<size_t>::ItemPtr FileStoreTemplate<size_t>::find(Item
template<typename size_t>
typename FileStoreTemplate<size_t>::ItemPtr FileStoreTemplate<size_t>::find(InodeId_t id) const {
+ oxTrace("ox::fs::FileStoreTemplate::find") << "Searching for inode:" << id;
auto fsData = fileStoreData();
if (fsData) {
auto root = m_buffer->ptr(fsData->rootNode);
diff --git a/deps/ox/src/ox/fs/filesystem/directory.hpp b/deps/ox/src/ox/fs/filesystem/directory.hpp
index a2dbc63d..741ee28b 100644
--- a/deps/ox/src/ox/fs/filesystem/directory.hpp
+++ b/deps/ox/src/ox/fs/filesystem/directory.hpp
@@ -133,13 +133,13 @@ ox::Error Directory<FileStore, InodeId_t>::init() noexcept {
oxTrace("ox::fs::Directory::init") << "Initializing Directory with Inode ID:" << m_inodeId;
oxReturnError(m_fs.write(m_inodeId, nullptr, Size));
auto buff = m_fs.read(m_inodeId).template to<Buffer>();
- if (buff.valid()) {
- new (buff) Buffer(Size);
- m_size = Size;
- return OxError(0);
+ if (!buff.valid()) {
+ m_size = 0;
+ return OxError(1);
}
- m_size = 0;
- return OxError(1);
+ new (buff) Buffer(Size);
+ m_size = Size;
+ return OxError(0);
}
template<typename FileStore, typename InodeId_t>
@@ -217,25 +217,27 @@ ox::Error Directory<FileStore, InodeId_t>::write(PathIterator path, InodeId_t in
return OxError(1);
}
} else {
+ oxTrace("ox::fs::Directory::write") << path.fullPath();
// insert the new entry on this directory
// get the name
path.next(name);
- oxTrace("ox::fs::Directory::write") << "Attempting to write Directory to FileStore";
-
// find existing version of directory
- oxTrace("ox::fs::Directory::write") << "Searching for inode" << m_inodeId;
- auto old = m_fs.read(m_inodeId);
+ oxTrace("ox::fs::Directory::write") << "Searching for directory inode" << m_inodeId;
+ auto oldStat = m_fs.stat(m_inodeId);
+ oxReturnError(oldStat);
+ oxTrace("ox::fs::Directory::write") << "Found existing directory of size" << oldStat.value.size;
+ auto old = m_fs.read(m_inodeId).template to<Buffer>();
if (!old.valid()) {
oxTrace("ox::fs::Directory::write::fail") << "Could not read existing version of Directory";
return OxError(1);
}
- const auto entrySize = DirectoryEntry<InodeId_t>::spaceNeeded(name->len() + 1); // add 1 for \0
- const auto entryDataSize = DirectoryEntry<InodeId_t>::DirectoryEntryData::spaceNeeded(name->len());
+ const auto entryDataSize = DirectoryEntry<InodeId_t>::DirectoryEntryData::spaceNeeded(name->len() + 1);
+ const auto entrySize = DirectoryEntry<InodeId_t>::spaceNeeded(entryDataSize);
const auto newSize = Buffer::spaceNeeded(m_size + entrySize);
- auto cpy = ox_malloca(newSize, Buffer, old);
+ auto cpy = ox_malloca(newSize, Buffer, *old, oldStat.value.size);
if (cpy == nullptr) {
oxTrace("ox::fs::Directory::write::fail") << "Could not allocate memory for copy of Directory";
return OxError(1);
@@ -346,16 +348,14 @@ ValErr<typename FileStore::InodeId_t> Directory<FileStore, InodeId_t>::find(Path
oxReturnError(path.get(name));
auto v = findEntry(name->c_str());
- if (!v.error) {
- return v;
+ oxReturnError(v);
+ // recurse if not at end of path
+ if (auto p = path.next(); p.valid()) {
+ Directory dir(m_fs, v.value);
+ name = nullptr;
+ return dir.find(p, nameBuff);
}
- name = nullptr;
- v = find(path.next(), nameBuff);
- if (!v.error) {
- return v;
- }
-
- return {0, OxError(1)};
+ return v;
}
diff --git a/deps/ox/src/ox/fs/filesystem/filesystem.cpp b/deps/ox/src/ox/fs/filesystem/filesystem.cpp
index a776617b..32deb84d 100644
--- a/deps/ox/src/ox/fs/filesystem/filesystem.cpp
+++ b/deps/ox/src/ox/fs/filesystem/filesystem.cpp
@@ -10,7 +10,7 @@
namespace ox {
-[[nodiscard]] ox::ValErr<const uint8_t*> FileSystem::read(FileAddress addr) {
+[[nodiscard]] ox::ValErr<uint8_t*> FileSystem::read(FileAddress addr) {
switch (addr.type()) {
case FileAddressType::Inode:
return read(addr.getInode().value);
diff --git a/deps/ox/src/ox/fs/filesystem/filesystem.hpp b/deps/ox/src/ox/fs/filesystem/filesystem.hpp
index 10282f3b..c81eb29c 100644
--- a/deps/ox/src/ox/fs/filesystem/filesystem.hpp
+++ b/deps/ox/src/ox/fs/filesystem/filesystem.hpp
@@ -32,19 +32,19 @@ class FileSystem {
[[nodiscard]] virtual ox::Error read(const char *path, void *buffer, std::size_t buffSize) = 0;
- [[nodiscard]] virtual ox::ValErr<const uint8_t*> read(const char *path) = 0;
+ [[nodiscard]] virtual ox::ValErr<uint8_t*> read(const char *path) = 0;
[[nodiscard]] virtual ox::Error read(uint64_t inode, void *buffer, std::size_t size) = 0;
[[nodiscard]] virtual ox::Error read(uint64_t inode, std::size_t readStart, std::size_t readSize, void *buffer, std::size_t *size) = 0;
- [[nodiscard]] virtual ox::ValErr<const uint8_t*> read(uint64_t inode) = 0;
+ [[nodiscard]] virtual ox::ValErr<uint8_t*> read(uint64_t inode) = 0;
[[nodiscard]] ox::Error read(FileAddress addr, void *buffer, std::size_t size);
[[nodiscard]] ox::Error read(FileAddress addr, std::size_t readStart, std::size_t readSize, void *buffer, std::size_t *size);
- [[nodiscard]] ox::ValErr<const uint8_t*> read(FileAddress addr);
+ [[nodiscard]] ox::ValErr<uint8_t*> read(FileAddress addr);
[[nodiscard]] virtual ox::Error remove(const char *path, bool recursive = false) = 0;
@@ -110,13 +110,13 @@ class FileSystemTemplate: public FileSystem {
[[nodiscard]] ox::Error read(const char *path, void *buffer, std::size_t buffSize) override;
- [[nodiscard]] ox::ValErr<const uint8_t*> read(const char*) override;
+ [[nodiscard]] ox::ValErr<uint8_t*> read(const char*) override;
[[nodiscard]] ox::Error read(uint64_t inode, void *buffer, std::size_t size) override;
[[nodiscard]] ox::Error read(uint64_t inode, std::size_t readStart, std::size_t readSize, void *buffer, std::size_t *size) override;
- [[nodiscard]] ox::ValErr<const uint8_t*> read(uint64_t) override;
+ [[nodiscard]] ox::ValErr<uint8_t*> read(uint64_t) override;
template<typename F>
[[nodiscard]] ox::Error ls(const char *dir, F cb);
@@ -185,8 +185,9 @@ ox::Error FileSystemTemplate<FileStore, Directory>::format(void *buff, uint64_t
oxTrace("ox::fs::FileSystemTemplate::format") << "rootDirInode:" << fd.rootDirInode;
oxReturnError(fs.write(InodeFsData, &fd, sizeof(fd)));
- if (fs.read(fd.rootDirInode).valid()) {
+ if (!fs.read(fd.rootDirInode).valid()) {
oxTrace("ox::fs::FileSystemTemplate::format::error") << "FileSystemTemplate::format did not correctly create root directory";
+ return OxError(1);
}
return OxError(0);
@@ -223,7 +224,7 @@ ox::Error FileSystemTemplate<FileStore, Directory>::read(const char *path, void
}
template<typename FileStore, typename Directory>
-[[nodiscard]] ox::ValErr<const uint8_t*> FileSystemTemplate<FileStore, Directory>::read(const char *path) {
+[[nodiscard]] ox::ValErr<uint8_t*> FileSystemTemplate<FileStore, Directory>::read(const char *path) {
auto fd = fileSystemData();
oxReturnError(fd.error);
Directory rootDir(m_fs, fd.value.rootDirInode);
@@ -243,7 +244,7 @@ ox::Error FileSystemTemplate<FileStore, Directory>::read(uint64_t inode, std::si
}
template<typename FileStore, typename Directory>
-[[nodiscard]] ox::ValErr<const uint8_t*> FileSystemTemplate<FileStore, Directory>::read(uint64_t inode) {
+[[nodiscard]] ox::ValErr<uint8_t*> FileSystemTemplate<FileStore, Directory>::read(uint64_t inode) {
auto data = m_fs.read(inode);
if (!data.valid()) {
return OxError(1);
diff --git a/deps/ox/src/ox/fs/filesystem/passthroughfs.cpp b/deps/ox/src/ox/fs/filesystem/passthroughfs.cpp
index cf8d5884..6afc1ef1 100644
--- a/deps/ox/src/ox/fs/filesystem/passthroughfs.cpp
+++ b/deps/ox/src/ox/fs/filesystem/passthroughfs.cpp
@@ -58,7 +58,7 @@ Error PassThroughFS::read(const char *path, void *buffer, std::size_t buffSize)
return OxError(1);
}
-ValErr<const uint8_t*> PassThroughFS::read(const char*) {
+ValErr<uint8_t*> PassThroughFS::read(const char*) {
return OxError(1);
}
@@ -72,7 +72,7 @@ Error PassThroughFS::read(uint64_t, std::size_t, std::size_t, void*, std::size_t
return OxError(1);
}
-ValErr<const uint8_t*> PassThroughFS::read(uint64_t) {
+ValErr<uint8_t*> PassThroughFS::read(uint64_t) {
return OxError(1);
}
diff --git a/deps/ox/src/ox/fs/filesystem/passthroughfs.hpp b/deps/ox/src/ox/fs/filesystem/passthroughfs.hpp
index 64bbd3c7..5456d2c8 100644
--- a/deps/ox/src/ox/fs/filesystem/passthroughfs.hpp
+++ b/deps/ox/src/ox/fs/filesystem/passthroughfs.hpp
@@ -43,13 +43,13 @@ class PassThroughFS: public FileSystem {
ox::Error read(const char *path, void *buffer, std::size_t buffSize) override;
- ox::ValErr<const uint8_t*> read(const char*) override;
+ ox::ValErr<uint8_t*> read(const char*) override;
ox::Error read(uint64_t inode, void *buffer, std::size_t size) override;
ox::Error read(uint64_t inode, std::size_t readStart, std::size_t readSize, void *buffer, std::size_t *size) override;
- ox::ValErr<const uint8_t*> read(uint64_t) override;
+ ox::ValErr<uint8_t*> read(uint64_t) override;
template<typename F>
ox::Error ls(const char *dir, F cb);
diff --git a/deps/ox/src/ox/fs/filesystem/pathiterator.cpp b/deps/ox/src/ox/fs/filesystem/pathiterator.cpp
index 42186b9c..3d0a9191 100644
--- a/deps/ox/src/ox/fs/filesystem/pathiterator.cpp
+++ b/deps/ox/src/ox/fs/filesystem/pathiterator.cpp
@@ -8,6 +8,7 @@
#include <ox/std/memops.hpp>
#include <ox/std/strops.hpp>
+#include <ox/std/trace.hpp>
#include "pathiterator.hpp"
namespace ox {
@@ -59,33 +60,37 @@ Error PathIterator::fileName(char *out, std::size_t outSize) {
// Gets the get item in the path
Error PathIterator::get(char *pathOut, std::size_t pathOutSize) {
std::size_t size = 0;
- auto retval = OxError(1);
- if (m_iterator < m_maxSize && ox_strlen(&m_path[m_iterator])) {
- retval = OxError(0);
- auto start = m_iterator;
- if (m_path[start] == '/') {
- start++;
- }
- // end is at the next /
- const char *substr = ox_strchr(&m_path[start], '/', m_maxSize - start);
- // correct end if it is invalid, which happens if there is no next /
- if (!substr) {
- substr = ox_strchr(&m_path[start], 0, m_maxSize - start);
- }
- std::size_t end = substr - m_path;
- size = end - start;
- // cannot fit the output in the output parameter
- if (size >= pathOutSize || size == 0) {
- return OxError(1);
- }
- ox_memcpy(pathOut, &m_path[start], size);
+ if (m_iterator >= m_maxSize) {
+ oxTrace("ox::fs::PathIterator::get") << "m_iterator >= m_maxSize";
+ return OxError(1);
}
+ if (!ox_strlen(&m_path[m_iterator])) {
+ oxTrace("ox::fs::PathIterator::get") << "!ox_strlen(&m_path[m_iterator])";
+ return OxError(1);
+ }
+ auto start = m_iterator;
+ if (m_path[start] == '/') {
+ start++;
+ }
+ // end is at the next /
+ const char *substr = ox_strchr(&m_path[start], '/', m_maxSize - start);
+ // correct end if it is invalid, which happens if there is no next /
+ if (!substr) {
+ substr = ox_strchr(&m_path[start], 0, m_maxSize - start);
+ }
+ std::size_t end = substr - m_path;
+ size = end - start;
+ // cannot fit the output in the output parameter
+ if (size >= pathOutSize || size == 0) {
+ return OxError(1);
+ }
+ ox_memcpy(pathOut, &m_path[start], size);
// truncate trailing /
if (size && pathOut[size - 1] == '/') {
size--;
}
pathOut[size] = 0; // end with null terminator
- return retval;
+ return OxError(0);
}
// Gets the get item in the path