From 9541287d4a76980f1abc22a26987ec8488fa2e6f Mon Sep 17 00:00:00 2001 From: Gary Talent Date: Thu, 12 Apr 2018 00:02:09 -0500 Subject: [PATCH] [ox/fs] Fix a read/write issue in new FileStore --- deps/ox/src/ox/__buildinfo/buildinfo.cpp | 37 +++++++++++ deps/ox/src/ox/__buildinfo/buildinfo.hpp | 17 +++++ .../src/ox/fs/filestore/filestoretemplate.hpp | 53 +++++++++++---- deps/ox/src/ox/fs/filestore/nodebuffer.hpp | 65 ++++++++++--------- deps/ox/src/ox/fs/filestore/ptr.hpp | 50 +++++++------- deps/ox/src/ox/fs/test/tests.cpp | 29 +++++++-- 6 files changed, 173 insertions(+), 78 deletions(-) create mode 100644 deps/ox/src/ox/__buildinfo/buildinfo.cpp create mode 100644 deps/ox/src/ox/__buildinfo/buildinfo.hpp diff --git a/deps/ox/src/ox/__buildinfo/buildinfo.cpp b/deps/ox/src/ox/__buildinfo/buildinfo.cpp new file mode 100644 index 00000000..eb25069f --- /dev/null +++ b/deps/ox/src/ox/__buildinfo/buildinfo.cpp @@ -0,0 +1,37 @@ +/* + * Copyright 2015 - 2018 gtalent2@gmail.com + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +namespace ox::buildinfo { + +#if defined(OX_USE_STDLIB) +const auto UseStdLib = true; +#else +const auto UseStdLib = false; +#endif + +#if defined(DEBUG) +const auto Debug = true; +#else +const auto Debug = false; +#endif + +#if defined(NDEBUG) +const auto NDebug = true; +#else +const auto NDebug = false; +#endif + +#if defined(__BIG_ENDIAN__) +const auto BigEndian = true; +const auto LittleEndian = false; +#else +const auto BigEndian = false; +const auto LittleEndian = true; +#endif + +} diff --git a/deps/ox/src/ox/__buildinfo/buildinfo.hpp b/deps/ox/src/ox/__buildinfo/buildinfo.hpp new file mode 100644 index 00000000..ef219070 --- /dev/null +++ b/deps/ox/src/ox/__buildinfo/buildinfo.hpp @@ -0,0 +1,17 @@ +/* + * Copyright 2015 - 2018 gtalent2@gmail.com + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +namespace ox::buildinfo { + +extern const bool UseStdLib; +extern const bool Debug; +extern const bool NDebug; +extern const bool BigEndian; +extern const bool LittleEndian; + +} diff --git a/deps/ox/src/ox/fs/filestore/filestoretemplate.hpp b/deps/ox/src/ox/fs/filestore/filestoretemplate.hpp index 0c1f91fc..a6927e37 100644 --- a/deps/ox/src/ox/fs/filestore/filestoretemplate.hpp +++ b/deps/ox/src/ox/fs/filestore/filestoretemplate.hpp @@ -127,7 +127,7 @@ class FileStoreTemplate: public FileStore { template FileStoreTemplate::FileStoreTemplate(void *buff, size_t buffSize) { m_buffSize = buffSize; - m_buffer = static_cast>*>(buff); + m_buffer = reinterpret_cast>*>(buff); if (!m_buffer->valid(buffSize)) { m_buffSize = 0; m_buffer = nullptr; @@ -137,10 +137,15 @@ FileStoreTemplate::FileStoreTemplate(void *buff, size_t buffSize) { template Error FileStoreTemplate::format() { new (m_buffer) Buffer(m_buffSize); - auto data = m_buffer->malloc(sizeof(FileStoreData)); - if (data.valid()) { - new (data->data()) FileStoreData; - return 0; + auto fsData = m_buffer->malloc(sizeof(FileStoreData)); + if (fsData.valid()) { + auto data = m_buffer->template dataOf(fsData); + if (data.valid()) { + new (data) FileStoreData; + return 0; + } else { + oxTrace("ox::fs::FileStoreTemplate::format::fail") << "Could not read data section of FileStoreData"; + } } return 1; } @@ -175,6 +180,7 @@ Error FileStoreTemplate::decLinks(InodeId_t id) { template Error FileStoreTemplate::write(InodeId_t id, void *data, FsSize_t dataSize, uint8_t fileType) { + oxTrace("ox::fs::FileStoreTemplate::write") << "Attempting to write to inode" << id; auto existing = find(id); // TODO: try compacting if unable to write if (canWrite(existing, dataSize)) { @@ -192,18 +198,19 @@ Error FileStoreTemplate::write(InodeId_t id, void *data, FsSize_t dataSi new (dest) FileStoreItem(dataSize); dest->id = id; dest->fileType = fileType; - auto destData = dest->data(); + auto destData = m_buffer->template dataOf(dest); if (destData.valid()) { ox_memcpy(destData, data, dest->size()); auto fsData = fileStoreData(); if (fsData) { auto root = m_buffer->ptr(fsData->rootNode); if (root.valid()) { - oxTrace("ox::fs::FileStoreTemplate::write") << "Placing" << dest->id << "on" << root->id; + oxTrace("ox::fs::FileStoreTemplate::write") << "Placing" << dest->id << "on" << root->id << "at" << destData.offset(); return placeItem(root, dest); } else { - oxTrace("ox::fs::FileStoreTemplate::write") << "Initializing root inode."; - fsData->rootNode = dest; + oxTrace("ox::fs::FileStoreTemplate::write") << "Initializing root inode with offset of" << dest.offset() + << "and data size of" << destData.size(); + fsData->rootNode = dest.offset(); return 0; } } else { @@ -218,16 +225,26 @@ Error FileStoreTemplate::write(InodeId_t id, void *data, FsSize_t dataSi template Error FileStoreTemplate::read(InodeId_t id, void *data, FsSize_t dataSize, FsSize_t *size) { + oxTrace("ox::fs::FileStoreTemplate::read") << "Attempting to read from inode" << id; auto src = find(id); if (src.valid()) { - auto srcData = src->data(); + auto srcData = m_buffer->template dataOf(src); + + oxTrace("ox::fs::FileStoreTemplate::read::found") << id << "found at"<< src.offset() + << "with data section at" << srcData.offset(); + oxTrace("ox::fs::FileStoreTemplate::read::outSize") << srcData.offset() << srcData.size() << dataSize; if (srcData.valid() && srcData.size() <= dataSize) { ox_memcpy(data, srcData, srcData.size()); if (size) { *size = src.size(); } return 0; + } else { + oxTrace("ox::fs::FileStoreTemplate::read::fail") << "Could not read data section of item:" << id; + oxTrace("ox::fs::FileStoreTemplate::read::fail") << "Item data section size:" << srcData.size(); } + } else { + oxTrace("ox::fs::FileStoreTemplate::read::fail") << "Could not find requested item:" << id; } return 1; } @@ -238,15 +255,21 @@ Error FileStoreTemplate::read(InodeId_t id, FsSize_t readStart, FsSize_t if (src.valid()) { auto srcData = src->data(); if (srcData.valid()) { - auto sub = srcData.subPtr(readStart, readSize); + auto sub = srcData.template subPtr(readStart, readSize); if (sub.valid()) { ox_memcpy(data, sub, sub.size()); if (size) { *size = sub.size(); } return 0; + } else { + oxTrace("ox::fs::FileStoreTemplate::read::fail") << "Could not read requested data sub-section of item:" << id; } + } else { + oxTrace("ox::fs::FileStoreTemplate::read::fail") << "Could not read data section of item:" << id; } + } else { + oxTrace("ox::fs::FileStoreTemplate::read::fail") << "Could not find requested item:" << id; } return 1; } @@ -266,7 +289,7 @@ int FileStoreTemplate::read(InodeId_t id, FsSize_t readStart, // copying to final destination T tmp; for (size_t i = 0; i < sizeof(T); i++) { - *reinterpret_cast(&tmp)[i] = *(reinterpret_cast(sub.get()) + i); + *(reinterpret_cast(&tmp)[i]) = *(reinterpret_cast(sub.get()) + i); } *(data + i) = tmp; } @@ -327,7 +350,7 @@ Error FileStoreTemplate::placeItem(ItemPtr root, ItemPtr item, int depth if (item->id > root->id) { auto right = m_buffer->ptr(root->right); if (!right.valid() || right->id == item->id) { - root->right = root; + root->right = root.offset(); return 0; } else { return placeItem(right, item, depth + 1); @@ -335,7 +358,7 @@ Error FileStoreTemplate::placeItem(ItemPtr root, ItemPtr item, int depth } else if (item->id < root->id) { auto left = m_buffer->ptr(root->left); if (!left.valid() || left->id == item->id) { - root->left = root; + root->left = root.offset(); return 0; } else { return placeItem(left, item, depth + 1); @@ -382,6 +405,8 @@ typename FileStoreTemplate::ItemPtr FileStoreTemplate::find(Item } else { return item; } + } else { + oxTrace("ox::fs::FileStoreTemplate::find::fail") << "item invalid"; } } else { oxTrace("ox::fs::FileStoreTemplate::find::fail") << "Excessive recursion depth, stopping before stack overflow. Search for:" << id; diff --git a/deps/ox/src/ox/fs/filestore/nodebuffer.hpp b/deps/ox/src/ox/fs/filestore/nodebuffer.hpp index b146f5ff..60165725 100644 --- a/deps/ox/src/ox/fs/filestore/nodebuffer.hpp +++ b/deps/ox/src/ox/fs/filestore/nodebuffer.hpp @@ -24,30 +24,7 @@ class __attribute__((packed)) NodeBuffer { ox::LittleEndian firstItem = 0; }; - struct ItemPtr: public ox::fs::Ptr { - inline ItemPtr() = default; - - inline ItemPtr(std::nullptr_t) { - } - - inline ItemPtr(void *dataStart, size_t dataSize, size_t itemOffset, size_t size): - Ptr(dataStart, dataSize, itemOffset, size) { - } - - inline ItemPtr(void *dataStart, size_t dataSize, size_t itemOffset) { - // make sure this can be read as an Item, and then use Item::size for the size - auto itemSpace = dataSize - itemOffset; - auto item = reinterpret_cast(static_cast(dataStart) + itemOffset); - if (itemOffset >= sizeof(Header) and - itemOffset < dataSize - sizeof(Item) and - itemSpace >= static_cast(sizeof(Item)) and - itemSpace >= item->fullSize()) { - this->init(dataStart, dataSize, itemOffset, sizeof(item) + item->fullSize()); - } else { - this->init(dataStart, dataSize, 0, 0); - } - } - }; + using ItemPtr = ox::fs::Ptr; Header m_header; @@ -60,6 +37,12 @@ class __attribute__((packed)) NodeBuffer { ItemPtr lastItem(); + /** + * @return the data section of the given item + */ + template + Ptr dataOf(ItemPtr); + ItemPtr prev(Item *item); ItemPtr next(Item *item); @@ -115,6 +98,14 @@ typename NodeBuffer::ItemPtr NodeBuffer::lastItem() return nullptr; } +template +template +Ptr NodeBuffer::dataOf(ItemPtr ip) { + auto out = ip.template subPtr(sizeof(Item)); + oxAssert(out.size() == ip.size() - sizeof(Item), "Sub Ptr has invalid size."); + return out; +} + template typename NodeBuffer::ItemPtr NodeBuffer::prev(Item *item) { return ptr(item->prev); @@ -126,8 +117,18 @@ typename NodeBuffer::ItemPtr NodeBuffer::next(Item * } template -typename NodeBuffer::ItemPtr NodeBuffer::ptr(size_t offset) { - return ItemPtr(this, m_header.size, offset); +typename NodeBuffer::ItemPtr NodeBuffer::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; + auto item = reinterpret_cast(reinterpret_cast(this) + itemOffset); + if (itemOffset >= sizeof(Header) and + itemOffset < m_header.size - sizeof(Item) and + itemSpace >= static_cast(sizeof(Item)) and + itemSpace >= item->fullSize()) { + return ItemPtr(this, m_header.size, itemOffset, item->fullSize()); + } else { + return ItemPtr(this, m_header.size, 0, 0); + } } template @@ -151,18 +152,18 @@ typename NodeBuffer::ItemPtr NodeBuffer::malloc(size new (out) Item(size); auto first = firstItem(); - out->next = first; + out->next = first.offset(); if (first.valid()) { - first->prev = out; + first->prev = out.offset(); } else { oxTrace("ox::fs::NodeBuffer::malloc::fail") << "NodeBuffer malloc failed due to invalid first element pointer."; return nullptr; } auto last = lastItem(); - out->prev = last; + out->prev = last.offset(); if (last.valid()) { - last->next = out; + last->next = out.offset(); } else { oxTrace("ox::fs::NodeBuffer::malloc::fail") << "NodeBuffer malloc failed due to invalid last element pointer."; return nullptr; @@ -182,12 +183,12 @@ void NodeBuffer::free(ItemPtr item) { auto prev = this->prev(item); auto next = this->next(item); if (prev.valid()) { - prev->next = next; + 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; + next->prev = prev.offset(); } else { oxTrace("ox::fs::NodeBuffer::free::fail") << "NodeBuffer free failed due to invalid next element pointer."; } diff --git a/deps/ox/src/ox/fs/filestore/ptr.hpp b/deps/ox/src/ox/fs/filestore/ptr.hpp index 7762f38e..7dbe3748 100644 --- a/deps/ox/src/ox/fs/filestore/ptr.hpp +++ b/deps/ox/src/ox/fs/filestore/ptr.hpp @@ -10,8 +10,7 @@ #include -namespace ox { -namespace fs { +namespace ox::fs { template class Ptr { @@ -49,11 +48,11 @@ class Ptr { inline T &operator*() const; - inline Ptr subPtr(size_t offset, size_t size); + template + inline Ptr subPtr(size_t offset, size_t size); - inline Ptr subPtr(size_t offset); - - inline void init(void *dataStart, size_t dataSize, size_t itemStart, size_t itemSize); + template + inline Ptr subPtr(size_t offset); }; @@ -63,7 +62,15 @@ inline Ptr::Ptr(std::nullptr_t) { template inline Ptr::Ptr(void *dataStart, size_t dataSize, size_t itemStart, size_t itemSize) { - init(dataStart, dataSize, itemStart, itemSize); + // do some sanity checks before assuming this is valid + if (itemSize >= sizeof(T) and + dataStart and + itemStart >= minOffset and + itemStart + itemSize <= dataSize) { + m_dataStart = reinterpret_cast(dataStart); + m_itemOffset = itemStart; + m_itemSize = itemSize; + } } template @@ -118,32 +125,21 @@ template inline T &Ptr::operator*() const { oxAssert(m_validated, "Unvalidated pointer dereference. (ox::fs::Ptr::operator*())"); oxAssert(valid(), "Invalid pointer dereference. (ox::fs::Ptr::operator*())"); - return *static_cast(this); + return *reinterpret_cast(this); } template -inline Ptr Ptr::subPtr(size_t offset, size_t size) { - auto dataSize = ((m_dataStart + offset) - m_dataStart) + m_itemSize; - return Ptr(m_dataStart, dataSize, offset, size); +template +inline Ptr Ptr::subPtr(size_t offset, size_t size) { + auto out = Ptr(get(), this->size(), offset, size); + return out; } template -inline Ptr Ptr::subPtr(size_t offset) { - return subPtr(offset, m_itemSize - offset); -} - -template -inline void Ptr::init(void *dataStart, size_t dataSize, size_t itemStart, size_t itemSize) { - // do some sanity checks before assuming this is valid - if (itemSize >= sizeof(T) and - dataStart and - itemStart >= minOffset and - itemStart + itemSize <= dataSize) { - m_dataStart = static_cast(dataStart); - m_itemOffset = itemStart; - m_itemSize = itemSize; - } +template +inline Ptr Ptr::subPtr(size_t offset) { + oxTrace("ox::fs::Ptr::subPtr") << m_itemOffset << this->size() << offset << m_itemSize << (m_itemSize - offset); + return subPtr(offset, m_itemSize - offset); } } -} diff --git a/deps/ox/src/ox/fs/test/tests.cpp b/deps/ox/src/ox/fs/test/tests.cpp index 0536e550..75329409 100644 --- a/deps/ox/src/ox/fs/test/tests.cpp +++ b/deps/ox/src/ox/fs/test/tests.cpp @@ -332,6 +332,19 @@ map tests = { return retval; } }, + { + "Ptr::subPtr", + [](string) { + constexpr auto buffLen = 5000; + uint8_t buff[buffLen]; + ox::fs::Ptr p(buff, buffLen, 500, 500); + oxAssert(p.valid(), "Ptr::subPtr: Ptr p is invalid."); + + auto subPtr = p.subPtr(50); + oxAssert(subPtr.valid(), "Ptr::subPtr: Ptr subPtr is invalid."); + return 0; + } + }, { "NodeBuffer::insert", [](string) { @@ -339,7 +352,8 @@ map tests = { constexpr auto buffLen = 5000; uint8_t buff[buffLen]; auto list = new (buff) ox::fs::NodeBuffer>(buffLen); - err |= !(list->malloc(50).valid()); + oxAssert(list->malloc(50).valid(), "NodeBuffer::insert: malloc 1 failed"); + oxAssert(list->malloc(50).valid(), "NodeBuffer::insert: malloc 2 failed"); auto first = list->firstItem(); oxAssert(first.valid(), "NodeBuffer::insert: Could not access first item"); oxAssert(first->size() == 50, "NodeBuffer::insert: First item size invalid"); @@ -351,15 +365,20 @@ map tests = { [](string) { constexpr auto buffLen = 5000; constexpr auto str1 = "Hello, World!"; - constexpr auto str1Len = ox_strlen(str1); + constexpr auto str1Len = ox_strlen(str1) + 1; constexpr auto str2 = "Hello, Moon!"; - constexpr auto str2Len = ox_strlen(str2); + constexpr auto str2Len = ox_strlen(str2) + 1; uint8_t buff[buffLen]; auto list = new (buff) ox::fs::NodeBuffer>(buffLen); ox::fs::FileStore32 fileStore(list, buffLen); oxAssert(fileStore.format() == 0, "FileStore::format failed."); - oxAssert(fileStore.write(4, (void*) str1, str1Len, 1) == 0, "FileStore::write 1 failed."); - oxAssert(fileStore.write(5, (void*) str2, str2Len, 1) == 0, "FileStore::write 2 failed."); + oxAssert(fileStore.write(4, const_cast(str1), str1Len, 1) == 0, "FileStore::write 1 failed."); + oxAssert(fileStore.write(5, const_cast(str2), str2Len, 1) == 0, "FileStore::write 2 failed."); + + char str1Read[str1Len]; + size_t str1ReadSize = 0; + oxAssert(fileStore.read(4, reinterpret_cast(str1Read), str1Len, &str1ReadSize) == 0, "FileStore::read 1 failed."); + return 0; } },