[ox] Fix OxFS 2 Directory test

deps/ox/src/ox/fs/filestore/filestoretemplate.hpp

@@ -208,7 +208,12 @@ Error FileStoreTemplate<size_t>::write(InodeId_t id, void *data, FsSize_t dataSi
 	if (canWrite(existing, dataSize)) {
 		// delete the old node if it exists
 		if (existing.valid()) {
-			m_buffer->free(existing);
+			oxTrace("ox::fs::FileStoreTemplate::write") << "Freeing old version of inode found at offset:" << existing.offset();
+			auto err = m_buffer->free(existing);
+			if (err) {
+				oxTrace("ox::fs::FileStoreTemplate::write::fail") << "Free of old version of inode failed";
+				return err;
+			}
 			existing = nullptr;
 		}
 
@@ -220,7 +225,6 @@ Error FileStoreTemplate<size_t>::write(InodeId_t id, void *data, FsSize_t dataSi
 			dest = m_buffer->malloc(dataSize);
 		}
 		if (dest.valid()) {
-			new (dest) FileStoreItem<size_t>(dataSize);
 			dest->id = id;
 			dest->fileType = fileType;
 			auto destData = m_buffer->template dataOf<uint8_t>(dest);
@@ -507,7 +511,7 @@ typename FileStoreTemplate<size_t>::ItemPtr FileStoreTemplate<size_t>::find(Item
 				return find(m_buffer->ptr(item->right), id, depth + 1);
 			} else if (id < item->id) {
 				return find(m_buffer->ptr(item->left), id, depth + 1);
-			} else {
+			} else if (id == item->id) {
 				return item;
 			}
 		} else {

deps/ox/src/ox/fs/filesystem/pathiterator.hpp

@@ -12,7 +12,8 @@
 
 namespace ox {
 
-constexpr auto MaxFileNameLength = 255;
+constexpr std::size_t MaxFileNameLength = 255;
+using FileName = BString<MaxFileNameLength>;
 
 class PathIterator {
 	private:
@@ -48,12 +49,12 @@ class PathIterator {
 		/**
 		 * @return 0 if no error
 		 */
-		Error next(BString<MaxFileNameLength> *fileName);
+		Error next(FileName *fileName);
 
 		/**
 		 * @return 0 if no error
 		 */
-		Error get(BString<MaxFileNameLength> *fileName);
+		Error get(FileName *fileName);
 
 		/**
 		 * @return 0 if no error

deps/ox/src/ox/fs/filesystem2/directory.hpp

@@ -23,6 +23,11 @@ struct __attribute__((packed)) DirectoryEntry {
 			// DirectoryEntry fields
 			LittleEndian<InodeId_t> inode = 0;
 			char name[MaxFileNameLength];
+
+			static constexpr std::size_t spaceNeeded(std::size_t chars) {
+				return offsetof(DirectoryEntryData, name) + chars;
+			}
+
 		};
 
 		// NodeBuffer fields
@@ -37,16 +42,23 @@ struct __attribute__((packed)) DirectoryEntry {
 		DirectoryEntry() = default;
 
 		DirectoryEntry(InodeId_t inode, const char *name, InodeId_t bufferSize) {
+			init(inode, name, bufferSize);
+		}
+
+		Error init(InodeId_t inode, const char *name, InodeId_t bufferSize) {
+			m_bufferSize = bufferSize;
 			auto d = data();
 			if (d.valid()) {
 				d->inode = inode;
-				ox_strncpy(d->name, name, MaxFileNameLength);
+				ox_strncpy(d->name, name, ox::min(bufferSize, static_cast<InodeId_t>(MaxFileNameLength)));
-				m_bufferSize = bufferSize;
+				return OxError(0);
 			}
+			return OxError(1);
 		}
 
 		ptrarith::Ptr<DirectoryEntryData, InodeId_t> data() {
-			return ptrarith::Ptr<DirectoryEntryData, InodeId_t>(this, this->fullSize(), sizeof(*this), this->size());
+			oxTrace("ox::ptrarith::DirectoryEntry::data") << this->fullSize() << sizeof(*this) << this->size();
+			return ptrarith::Ptr<DirectoryEntryData, InodeId_t>(this, this->fullSize(), sizeof(*this), this->size(), this->size());
 		}
 
 		/**
@@ -57,14 +69,14 @@ struct __attribute__((packed)) DirectoryEntry {
 		}
 
 		InodeId_t size() const {
-			return fullSize() - offsetof(DirectoryEntryData, inode);
+			return fullSize() - sizeof(*this);
 		}
 
 		void setSize(InodeId_t) {
 			// ignore set value
 		}
 
-		static std::size_t spaceNeeded(std::size_t chars) {
+		static constexpr std::size_t spaceNeeded(std::size_t chars) {
 			return sizeof(DirectoryEntry) + offsetof(DirectoryEntryData, name) + chars;
 		}
 
@@ -89,11 +101,11 @@ class Directory {
 		 */
 		Error init() noexcept;
 
-		Error write(PathIterator it, InodeId_t inode) noexcept;
+		Error write(PathIterator it, InodeId_t inode, FileName *nameBuff = nullptr) noexcept;
 
 		Error rm(PathIterator it) noexcept;
 
-		ValErr<InodeId_t> find(const BString<MaxFileNameLength> &name) const noexcept;
+		ValErr<InodeId_t> find(const FileName &name) const noexcept;
 
 };
 
@@ -122,45 +134,53 @@ Error Directory<InodeId_t>::init() noexcept {
 }
 
 template<typename InodeId_t>
-Error Directory<InodeId_t>::write(PathIterator path, InodeId_t inode) noexcept {
+Error Directory<InodeId_t>::write(PathIterator path, InodeId_t inode, FileName *nameBuff) noexcept {
 	InodeId_t nextChild = 0;
+
+	// reuse nameBuff if it has already been allocated, as it is a rather large variable
+	if (nameBuff == nullptr) {
+		nameBuff = reinterpret_cast<FileName*>(ox_alloca(sizeof(FileName)));
+	}
+	auto name = nameBuff;
+
 	if ((path + 1).hasNext()) {
 		oxTrace("ox::fs::Directory::write") << "Attempting to write to next sub-Directory";
 
-		// read the name and pop it off the stack as soon as possible to help
+		path.get(name);
-		// avoid a stack overflow in the recursive calls
+		nextChild = find(*name);
-		{
-			BString<MaxFileNameLength> name;
-			path.get(&name);
-			nextChild = find(name);
-		}
 
 		// It's important for the recursive call to be outside the block where name
-		// lived. This avoids allocation several BString<MaxFileNameLength>s on the
+		// lived. This avoids allocation several FileNames on the
 		// stack at once, while also avoiding the use of heap memory.
 		if (nextChild) {
-			return Directory(m_fs, nextChild).write(path + 1, inode);
+			// reuse name because it is a rather large variable and will not be used again
+			// be attentive that this remains true
+			name = nullptr;
+			return Directory(m_fs, nextChild).write(path + 1, inode, nameBuff);
 		}
 	} else {
 		// insert the new entry on this directory
 
 		// get the name
-		BString<MaxFileNameLength> name;
+		path.next(name);
-		path.next(&name);
+
+		oxTrace("ox::fs::Directory::write") << "Attempting to write Directory to FileStore";
 
 		// find existing version of directory
 		auto old = m_fs->read(m_inodeId);
 		if (old.valid()) {
-			const auto newSize = m_size + DirectoryEntry<InodeId_t>::spaceNeeded(name.size());
+			const auto entrySize = DirectoryEntry<InodeId_t>::spaceNeeded(name->len());
+			const auto entryDataSize =DirectoryEntry<InodeId_t>::DirectoryEntryData::spaceNeeded(name->len());
+			const auto newSize = m_size + entrySize;
 			auto cpy = ox_malloca(newSize, Buffer, old);
 			if (cpy != nullptr) {
 				// TODO: look for old version of this entry and delete it
 
 				cpy->setSize(newSize);
-				auto val = cpy->malloc(name.size());
+				auto val = cpy->malloc(entryDataSize);
 				if (val.valid()) {
 					oxTrace("ox::fs::Directory::write") << "Attempting to write Directory to FileStore";
-					new (val) DirectoryEntry<InodeId_t>(inode, name.data(), newSize);
+					val->init(inode, name->data(), entrySize);
 					return m_fs->write(m_inodeId, cpy, cpy->size());
 				} else {
 					oxTrace("ox::fs::Directory::write::fail") << "Could not allocate memory for new directory entry";
@@ -181,17 +201,20 @@ Error Directory<InodeId_t>::rm(PathIterator) noexcept {
 }
 
 template<typename InodeId_t>
-ValErr<InodeId_t> Directory<InodeId_t>::find(const BString<MaxFileNameLength> &name) const noexcept {
+ValErr<InodeId_t> Directory<InodeId_t>::find(const FileName &name) const noexcept {
 	oxTrace("ox::fs::Directory::find") << name.c_str();
 	auto buff = m_fs->read(m_inodeId).template to<Buffer>();
 	if (buff.valid()) {
 		oxTrace("ox::fs::Directory::find") << "Found directory buffer.";
 		for (auto i = buff->iterator(); i.valid(); i.next()) {
 			auto data = i->data();
-			oxTrace("ox::fs::Directory::find::name") << name.c_str();
+			if (data.valid()) {
-			if (data.valid() && data->name == name.c_str()) {
+				if (ox_strncmp(data->name, name.c_str(), name.len()) == 0) {
 					return static_cast<InodeId_t>(data->inode);
 				}
+			} else {
+				oxTrace("ox::fs::Directory::find") << "INVALID DIRECTORY ENTRY";
+			}
 		}
 		oxTrace("ox::fs::Directory::find::fail");
 	} else {

deps/ox/src/ox/fs/test/tests.cpp

@@ -395,11 +395,11 @@ map<string, int(*)(string)> tests = {
 				oxAssert(dir->find("file1").error, "Could not find file1");
 				oxAssert(dir->find("file1") == 1, "Could not find file1");
 
-				//oxTrace("ox::fs::test::Directory") << "write 2";
+				oxTrace("ox::fs::test::Directory") << "write 2";
-				//oxAssert(dir->write("/file3", 3) == 0, "Directory write of file3 failed");
+				oxAssert(dir->write("/file3", 3) == 0, "Directory write of file3 failed");
 
-				//oxTrace("ox::fs::test::Directory") << "write 3";
+				oxTrace("ox::fs::test::Directory") << "write 3";
-				//oxAssert(dir->write("/file2", 2) == 0, "Directory write of file2 failed");
+				oxAssert(dir->write("/file2", 2) == 0, "Directory write of file2 failed");
 
 				return 0;
 			}

deps/ox/src/ox/mc/write.hpp

@@ -74,13 +74,13 @@ int MetalClawWriter::op(const char*, ox::BString<L> *val) {
 	if (val->len()) {
 		// write the length
 		typedef uint32_t StringLength;
-		if (m_buffIt + sizeof(StringLength) + val->size() < m_buffLen) {
+		if (m_buffIt + sizeof(StringLength) + val->bytes() < m_buffLen) {
-			*reinterpret_cast<LittleEndian<StringLength>*>(&m_buff[m_buffIt]) = static_cast<StringLength>(val->size());
+			*reinterpret_cast<LittleEndian<StringLength>*>(&m_buff[m_buffIt]) = static_cast<StringLength>(val->bytes());
 			m_buffIt += sizeof(StringLength);
 
 			// write the string
-			ox_memcpy(&m_buff[m_buffIt], val, val->size());
+			ox_memcpy(&m_buff[m_buffIt], val, val->bytes());
-			m_buffIt += val->size();
+			m_buffIt += val->bytes();
 			fieldSet = true;
 		} else {
 			err = MC_BUFFENDED;

deps/ox/src/ox/ptrarith/nodebuffer.hpp

@@ -43,6 +43,10 @@ class __attribute__((packed)) NodeBuffer {
 					return m_current;
 				}
 
+				Item *get() {
+					return m_current;
+				}
+
 				operator Item*() {
 					return m_current;
 				}
@@ -55,7 +59,7 @@ class __attribute__((packed)) NodeBuffer {
 					return m_current;
 				}
 
-				bool valid() {
+				bool valid() const noexcept {
 					return m_current.valid();
 				}
 
@@ -70,7 +74,11 @@ class __attribute__((packed)) NodeBuffer {
 
 				void next() {
 					oxTrace("ox::ptrarith::NodeBuffer::Iterator::next") << m_it++;
+					if (hasNext()) {
 						m_current = m_buffer->next(m_current);
+					} else {
+						m_current = nullptr;
+					}
 				}
 		};
 
@@ -107,7 +115,7 @@ class __attribute__((packed)) NodeBuffer {
 
 		ItemPtr malloc(size_t size);
 
-		void free(ItemPtr item);
+		Error free(ItemPtr item);
 
 		bool valid(size_t maxSize);
 
@@ -192,11 +200,10 @@ typename NodeBuffer<size_t, Item>::ItemPtr NodeBuffer<size_t, Item>::next(Item *
 template<typename size_t, typename Item>
 typename NodeBuffer<size_t, Item>::ItemPtr NodeBuffer<size_t, Item>::ptr(size_t itemOffset) {
 	// make sure this can be read as an Item, and then use Item::size for the size
-	auto itemSpace = m_header.size - itemOffset;
+	std::size_t itemSpace = m_header.size - itemOffset;
 	auto item = reinterpret_cast<Item*>(reinterpret_cast<uint8_t*>(this) + itemOffset);
 	if (itemOffset >= sizeof(Header) &&
-		 itemOffset < m_header.size - sizeof(Item) &&
+		 itemSpace >= sizeof(Item) &&
-		 itemSpace >= static_cast<size_t>(sizeof(Item)) &&
 		 itemSpace >= item->fullSize()) {
 		return ItemPtr(this, m_header.size, itemOffset, item->fullSize());
 	} else {
@@ -207,6 +214,7 @@ typename NodeBuffer<size_t, Item>::ItemPtr NodeBuffer<size_t, Item>::ptr(size_t
 
 template<typename size_t, typename Item>
 typename NodeBuffer<size_t, Item>::ItemPtr NodeBuffer<size_t, Item>::malloc(size_t size) {
+	oxTrace("ox::ptrarith::NodeBuffer::malloc") << "Size:" << size;
 	size_t fullSize = size + sizeof(Item);
 	if (m_header.size - m_header.bytesUsed >= fullSize) {
 		auto last = lastItem();
@@ -216,7 +224,7 @@ typename NodeBuffer<size_t, Item>::ItemPtr NodeBuffer<size_t, Item>::malloc(size
 		} else {
 			// there is no first item, so this may be the first item
 			if (!m_header.firstItem) {
-				oxTrace("ox::fs::NodeBuffer::malloc") << "No first item, initializing.";
+				oxTrace("ox::ptrarith::NodeBuffer::malloc") << "No first item, initializing.";
 				m_header.firstItem = sizeof(m_header);
 				addr = m_header.firstItem;
 			}
@@ -231,7 +239,7 @@ typename NodeBuffer<size_t, Item>::ItemPtr NodeBuffer<size_t, Item>::malloc(size
 			if (first.valid()) {
 				first->prev = out.offset();
 			} else {
-				oxTrace("ox::fs::NodeBuffer::malloc::fail") << "NodeBuffer malloc failed due to invalid first element pointer.";
+				oxTrace("ox::ptrarith::NodeBuffer::malloc::fail") << "NodeBuffer malloc failed due to invalid first element pointer.";
 				return nullptr;
 			}
 
@@ -240,34 +248,37 @@ typename NodeBuffer<size_t, Item>::ItemPtr NodeBuffer<size_t, Item>::malloc(size
 			if (last.valid()) {
 				last->next = out.offset();
 			} else {
-				oxTrace("ox::fs::NodeBuffer::malloc::fail") << "NodeBuffer malloc failed due to invalid last element pointer.";
+				oxTrace("ox::ptrarith::NodeBuffer::malloc::fail") << "NodeBuffer malloc failed due to invalid last element pointer.";
 				return nullptr;
 			}
 			m_header.bytesUsed += out.size();
 		} else {
-			oxTrace("ox::fs::NodeBuffer::malloc::fail") << "Unknown";
+			oxTrace("ox::ptrarith::NodeBuffer::malloc::fail") << "Unknown";
 		}
 		return out;
 	}
-	oxTrace("ox::fs::NodeBuffer::malloc::fail") << "Insufficient space:" << fullSize << "needed," << available() << "available";
+	oxTrace("ox::ptrarith::NodeBuffer::malloc::fail") << "Insufficient space:" << fullSize << "needed," << available() << "available";
 	return nullptr;
 }
 
 template<typename size_t, typename Item>
-void NodeBuffer<size_t, Item>::free(ItemPtr item) {
+Error NodeBuffer<size_t, Item>::free(ItemPtr item) {
 	auto prev = this->prev(item);
 	auto next = this->next(item);
-	if (prev.valid()) {
+	if (prev.valid() && next.valid()) {
 		prev->next = next.offset();
-	} else {
-		oxTrace("ox::fs::NodeBuffer::free::fail") << "NodeBuffer free failed due to invalid prev element pointer.";
-	}
-	if (next.valid()) {
 		next->prev = prev.offset();
 	} else {
-		oxTrace("ox::fs::NodeBuffer::free::fail") << "NodeBuffer free failed due to invalid next element pointer.";
+		if (!prev.valid()) {
+			oxTrace("ox::ptrarith::NodeBuffer::free::fail") << "NodeBuffer free failed due to invalid prev element pointer:" << prev.offset();
+		}
+		if (!next.valid()) {
+			oxTrace("ox::ptrarith::NodeBuffer::free::fail") << "NodeBuffer free failed due to invalid next element pointer:" << next.offset();
+		}
+		return OxError(1);
 	}
 	m_header.bytesUsed -= item.size();
+	return OxError(0);
 }
 
 template<typename size_t, typename Item>

deps/ox/src/ox/ptrarith/ptr.hpp

@@ -29,7 +29,7 @@ class Ptr {
 
 		inline Ptr(std::nullptr_t);
 
-		inline Ptr(void *dataStart, size_t dataSize, size_t itemStart, size_t itemSize = sizeof(T));
+		inline Ptr(void *dataStart, size_t dataSize, size_t itemStart, size_t itemSize = sizeof(T), size_t itemTypeSize = sizeof(T));
 
 		inline bool valid() const;
 
@@ -83,12 +83,12 @@ inline Ptr<T, size_t, minOffset>::Ptr(std::nullptr_t) {
 }
 
 template<typename T, typename size_t, size_t minOffset>
-inline Ptr<T, size_t, minOffset>::Ptr(void *dataStart, size_t dataSize, size_t itemStart, size_t itemSize) {
+inline Ptr<T, size_t, minOffset>::Ptr(void *dataStart, size_t dataSize, size_t itemStart, size_t itemSize, size_t itemTypeSize) {
 	// do some sanity checks before assuming this is valid
-	if (itemSize >= sizeof(T) &&
+	if (itemSize >= itemTypeSize &&
 	    dataStart &&
 	    itemStart >= minOffset &&
-	    static_cast<std::size_t>(itemStart + itemSize) <= dataSize) {
+	    itemStart + itemSize <= dataSize) {
 		m_dataStart = reinterpret_cast<uint8_t*>(dataStart);
 		m_dataSize = dataSize;
 		m_itemOffset = itemStart;

deps/ox/src/ox/std/string.hpp

@@ -51,7 +51,7 @@ class BString {
 		/**
 		 * Returns the number of bytes used for this string.
 		 */
-		std::size_t size() const noexcept;
+		std::size_t bytes() const noexcept;
 
 		/**
 		 * Returns the capacity of bytes for this string.
@@ -162,7 +162,7 @@ std::size_t BString<buffLen>::len() const noexcept {
 }
 
 template<std::size_t buffLen>
-std::size_t BString<buffLen>::size() const noexcept {
+std::size_t BString<buffLen>::bytes() const noexcept {
 	std::size_t i;
 	for (i = 0; i < buffLen && m_buff[i]; i++);
 	return i + 1; // add one for null terminator

deps/ox/src/ox/std/strops.hpp

@@ -53,6 +53,22 @@ constexpr int ox_strcmp(const char *str1, const char *str2) noexcept {
 	return retval;
 }
 
+constexpr int ox_strncmp(const char *str1, const char *str2, std::size_t len) noexcept {
+	auto retval = 0;
+	std::size_t i = 0;
+	while (i < len && (str1[i] || str2[i])) {
+		if (str1[i] < str2[i]) {
+			retval = -1;
+			break;
+		} else if (str1[i] > str2[i]) {
+			retval = 1;
+			break;
+		}
+		i++;
+	}
+	return retval;
+}
+
 constexpr const char *ox_strchr(const char *str, int character, std::size_t maxLen = 0xFFFFFFFF) noexcept {
 	for (std::size_t i = 0; i <= maxLen; i++) {
 		if (str[i] == character) {

deps/ox/src/ox/trace/trace.cpp

@@ -41,20 +41,13 @@ StdOutStream::StdOutStream(const char *file, int line, const char *ch, const cha
 
 StdOutStream::~StdOutStream() {
 #if defined(OX_USE_STDLIB)
-	std::cout << std::setw(50) << std::left << m_msg.ch.c_str() << '|';
+	std::cout << std::setw(53) << std::left << m_msg.ch.c_str() << '|';
-	std::cout << std::setw(60) << std::left << m_msg.msg.c_str() << '|';
+	std::cout << std::setw(65) << std::left << m_msg.msg.c_str() << '|';
 	std::cout << " " << m_msg.file.c_str() << ':' << m_msg.line << "\n";
 #endif
 }
 
 
-NullStream::NullStream(const char*, int, const char*, const char*) {
-}
-
-NullStream::~NullStream() {
-}
-
-
 void logError(const char *file, int line, Error err) {
 	if (err) {
 		ErrorInfo ei(err);

deps/ox/src/ox/trace/trace.hpp

@@ -66,7 +66,7 @@ class StdOutStream {
 		~StdOutStream();
 
 		template<typename T>
-		inline StdOutStream &operator<<(const T &v) {
+		constexpr inline StdOutStream &operator<<(const T &v) {
 			m_msg.msg += " ";
 			m_msg.msg += v;
 			return *this;
@@ -78,20 +78,25 @@ class StdOutStream {
 class NullStream {
 
 	public:
-		NullStream() = default;
+		constexpr NullStream() = default;
 
-		NullStream(const char *file, int line, const char *ch, const char *msg = "");
+		constexpr NullStream(const char*, int, const char*, const char* = "") {
+		}
 
-		~NullStream();
+		~NullStream() = default;
 
 		template<typename T>
-		inline NullStream &operator<<(const T&) {
+		constexpr inline NullStream &operator<<(const T&) {
 			return *this;
 		}
 
 };
 
+#ifdef DEBUG
 using TraceStream = StdOutStream;
+#else
+using TraceStream = NullStream;
+#endif
 
 void logError(const char *file, int line, Error err);