From ff3d44132962d77f6ce09222b1458ac210210931 Mon Sep 17 00:00:00 2001 From: Gary Talent Date: Thu, 16 Jun 2016 00:04:21 -0500 Subject: [PATCH] Fixed several problem with the file store. --- scripts/check | 1 + scripts/cibuild | 8 +++ scripts/newcpp | 16 +++++ scripts/setup_build | 8 +++ scripts/setup_build_debug | 8 +++ scripts/setup_build_gba | 8 +++ src/_memops.cpp | 2 +- src/_strops.cpp | 7 +- src/filestore.hpp | 136 +++++++++++++++++++++----------------- test/filestoreio.cpp | 24 +++++-- 10 files changed, 147 insertions(+), 71 deletions(-) create mode 100755 scripts/check create mode 100755 scripts/cibuild create mode 100755 scripts/newcpp create mode 100755 scripts/setup_build create mode 100755 scripts/setup_build_debug create mode 100755 scripts/setup_build_gba diff --git a/scripts/check b/scripts/check new file mode 100755 index 000000000..74346523e --- /dev/null +++ b/scripts/check @@ -0,0 +1 @@ +clang-check `find . | grep "\.cpp" | grep -v CMakeFiles | grep -v editormodels\.cpp` diff --git a/scripts/cibuild b/scripts/cibuild new file mode 100755 index 000000000..32577b927 --- /dev/null +++ b/scripts/cibuild @@ -0,0 +1,8 @@ +BRANCH=$1 +./scripts/setup_build +make -j4 -C build/sdl clean wombat +rm -rf wombat wombat-*-*.tar.gz +mkdir wombat +git rev-parse HEAD > wombat/revision.txt +cp -pr wombat_path build/sdl/src/wombat/wombat wombat +tar -cvzf wombat-${BRANCH}-`date "+%y%m%d%H%M"`.tar.gz wombat diff --git a/scripts/newcpp b/scripts/newcpp new file mode 100755 index 000000000..c9ef12ab5 --- /dev/null +++ b/scripts/newcpp @@ -0,0 +1,16 @@ +#! /usr/bin/env python + +import sys + +if len(sys.argv) < 3: + sys.exit(1) + +pkg = sys.argv[1] +name = sys.argv[2] +ifdef = "WOMBAT_%s_%s_HPP" % (pkg.upper(), name.upper()) +namespace = "namespace wombat {\nnamespace %s {\n\n}\n}" % pkg +hpp = "#ifndef %s\n#define %s\n\n%s\n\n#endif" % (ifdef, ifdef, namespace) +cpp = "#include \"%s.hpp\"\n\n%s" % (name, namespace) + +open("src/%s/%s.hpp" % (pkg, name), "w").write(hpp) +open("src/%s/%s.cpp" % (pkg, name), "w").write(cpp) diff --git a/scripts/setup_build b/scripts/setup_build new file mode 100755 index 000000000..bc6a14558 --- /dev/null +++ b/scripts/setup_build @@ -0,0 +1,8 @@ +#! /usr/bin/env bash +project=`pwd`/ +buildDir="build/sdl" + +mkdir -p $buildDir +pushd $buildDir +cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_BUILD_TYPE=Release $project +popd diff --git a/scripts/setup_build_debug b/scripts/setup_build_debug new file mode 100755 index 000000000..64ba2b525 --- /dev/null +++ b/scripts/setup_build_debug @@ -0,0 +1,8 @@ +#! /usr/bin/env bash +project=`pwd` +buildDir="build/sdl_debug" + +mkdir -p $buildDir +pushd $buildDir +cmake -DUSE_ASAN=ON -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_BUILD_TYPE=Debug $project +popd diff --git a/scripts/setup_build_gba b/scripts/setup_build_gba new file mode 100755 index 000000000..240a24f09 --- /dev/null +++ b/scripts/setup_build_gba @@ -0,0 +1,8 @@ +#! /usr/bin/env bash +project=`pwd` +buildDir="build/gba" + +mkdir -p $buildDir +pushd $buildDir +cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_TOOLCHAIN_FILE=cmake/Modules/GBA.cmake -DWOMBAT_BUILD_TYPE=GBA $project +popd diff --git a/src/_memops.cpp b/src/_memops.cpp index 072d05fa0..b81383370 100644 --- a/src/_memops.cpp +++ b/src/_memops.cpp @@ -10,7 +10,7 @@ namespace wombat { namespace fs { -void memcpy(void *src, void *dest, int size) { +void memcpy(void *dest, void *src, int size) { char *srcBuf = (char*) src; char *dstBuf = (char*) dest; for (int i = 0; i < size; i++) { diff --git a/src/_strops.cpp b/src/_strops.cpp index 31be2124d..c953b0994 100644 --- a/src/_strops.cpp +++ b/src/_strops.cpp @@ -10,14 +10,17 @@ namespace fs { int strcmp(const char *str1, const char *str2) { auto retval = 0; - for (int i = 0; str1[i] && str2[i]; i++) { + auto i = 0; + do { if (str1[i] < str2[i]) { retval = -1; break; } else if (str1[i] > str2[i]) { retval = 1; + break; } - } + i++; + } while (str1[i] == str2[i] && str1[i]); return retval; } diff --git a/src/filestore.hpp b/src/filestore.hpp index ef8371043..196050820 100644 --- a/src/filestore.hpp +++ b/src/filestore.hpp @@ -21,7 +21,7 @@ class FileStore { struct FsHeader { uint32_t version; FsSize_t size; - FsSize_t rootInode = 0; + FsSize_t rootInode; bool insertLeft = false; // crude tree balancing mechanism }; @@ -33,13 +33,15 @@ class FileStore { FsSize_t prev, next; FsSize_t left, right; FsSize_t dataLen; - // offsets from Inode this + + // The following variables should not be assumed to exist FsSize_t m_id; - FsSize_t m_data; + // must be last item + uint8_t m_data; FsSize_t size(); void setId(InodeId_t); - void setData(uint8_t *data, int size); + void setData(void *data, int size); private: Inode() = default; @@ -47,8 +49,6 @@ class FileStore { uint8_t *m_begin, *m_end; uint32_t &m_version; - // the last Inode in the FileStore's memory chunk - FsSize_t &m_lastRec; Inode *m_root; public: @@ -128,7 +128,7 @@ class FileStore { * If the record already exists, it replaces the old on deletes it. * @return true if the record was inserted */ - bool insert(Inode *root, Inode *insertValue, FsSize_t *rootParentPtr = 0); + bool insert(Inode *root, Inode *insertValue); /** * Gets the FsSize_t associated with the next Inode to be allocated. @@ -146,8 +146,12 @@ class FileStore { */ template T ptr(FsSize_t ptr) { - return (T) m_begin + ptr; + return (T) (m_begin + ptr); }; + + Inode *firstInode(); + + Inode *lastInode(); }; template @@ -161,16 +165,16 @@ void FileStore::Inode::setId(InodeId_t id) { } template -void FileStore::Inode::setData(uint8_t *data, int size) { - memcpy(this + m_data, data, size); - m_data = size; +void FileStore::Inode::setData(void *data, int size) { + memcpy(&m_data, data, size); + dataLen = size; } // FileStore template -FileStore::FileStore(uint8_t *begin, uint8_t *end, Error *error): m_version(*((uint32_t*) begin)), m_lastRec(*(FsSize_t*) (begin + sizeof(FsHeader))) { +FileStore::FileStore(uint8_t *begin, uint8_t *end, Error *error): m_version(*((uint32_t*) begin)) { if (version() != m_version) { // version mismatch if (error) { @@ -203,10 +207,11 @@ template int FileStore::write(InodeId_t id, void *data, FsSize_t dataLen) { auto retval = 1; const FsSize_t size = offsetof(Inode, m_id) + dataLen; - auto rec = (Inode*) alloc(size); - if (rec) { - rec->dataLen = dataLen; - if (insert(m_root, rec)) { + auto inode = (Inode*) alloc(size); + if (inode) { + inode->m_id = id; + inode->setData(data, dataLen); + if (insert(m_root, inode) || m_root == inode) { retval = 0; } } @@ -221,7 +226,7 @@ int FileStore::read(InodeId_t id, void *data, FsSize_t *size) { if (size) { *size = rec->dataLen; } - memcpy(data, ptr(rec->m_data), rec->dataLen); + memcpy(data, &rec->m_data, rec->dataLen); retval = 0; } return retval; @@ -234,37 +239,38 @@ typename FileStore::FsHeader *FileStore::getHeader() { template typename FileStore::Inode *FileStore::getRecord(FileStore::Inode *root, InodeId_t id) { - auto cmp = root->m_id > id; - FsSize_t recPt; - if (cmp) { - recPt = root->left; - } else if (!cmp) { - recPt = root->right; - } else { - recPt = ptr(root); - } - if (recPt) { - return getRecord(ptr(recPt), id); - } else { - return ptr(recPt); + Inode *retval = nullptr; + + if (root->m_id > id) { + if (root->left) { + retval = getRecord(ptr(root->left), id); + } + } else if (root->m_id < id) { + if (root->right) { + retval = getRecord(ptr(root->right), id); + } + } else if (root->m_id == id) { + retval = root; } + + return retval; } template void *FileStore::alloc(FsSize_t size) { - const auto iterator = this->iterator(); - if ((iterator + size) > (uint64_t) m_end) { + if ((lastInode()->next + size) > (uint64_t) m_end) { compress(); - if ((iterator + size) > (uint64_t) m_end) { + if ((lastInode()->next + size) > (uint64_t) m_end) { return nullptr; } } - ptr(m_lastRec)->next = iterator; - auto rec = ptr(iterator); + const auto retval = lastInode()->next; + const auto rec = ptr(retval); memset(rec, 0, size); - ptr(iterator)->prev = m_lastRec; - m_lastRec = iterator; + rec->prev = ptr(lastInode()); + rec->next = retval + size; + firstInode()->prev = retval; return rec; } @@ -282,41 +288,31 @@ void FileStore::compress() { } template -bool FileStore::insert(Inode *root, Inode *insertValue, FsSize_t *rootParentPtr) { - auto cmp = root->m_id > insertValue->m_id; - if (cmp) { +bool FileStore::insert(Inode *root, Inode *insertValue) { + auto retval = false; + + if (root->m_id > insertValue->m_id) { if (root->left) { - return insert(ptr(root->left), insertValue, &root->left); + retval = insert(ptr(root->left), insertValue); } else { - root->left = ((uint8_t*) insertValue) - m_begin; - return true; + root->left = ptr(insertValue); + retval = true; } - } else if (!cmp) { + } else if (root->m_id < insertValue->m_id) { if (root->right) { - return insert(ptr(root->right), insertValue, &root->right); + retval = insert(ptr(root->right), insertValue); } else { - root->right = ((uint8_t*) insertValue) - m_begin; - return true; + root->right = ptr(insertValue); + retval = true; } - } else { - auto ivAddr = ((uint8_t*) insertValue) - m_begin; - if (root->prev) { - ptr(root->prev)->next = ivAddr; - } - if (root->next) { - ptr(root->next)->prev = ivAddr; - } - if (rootParentPtr) { - *rootParentPtr = ivAddr; - } - return true; } - return false; + + return retval; } template FsSize_t FileStore::iterator() { - return m_lastRec + ((Inode*) m_begin + m_lastRec)->size(); + return ptr(lastInode()) + lastInode()->size(); } template @@ -324,6 +320,16 @@ FsSize_t FileStore::ptr(void *ptr) { return ((uint8_t*) ptr) - m_begin; } +template +typename FileStore::Inode *FileStore::firstInode() { + return ptr(sizeof(FsHeader)); +} + +template +typename FileStore::Inode *FileStore::lastInode() { + return ptr(firstInode()->prev); +} + template uint8_t FileStore::version() { return 0; @@ -331,9 +337,17 @@ uint8_t FileStore::version() { template uint8_t *FileStore::format(uint8_t *buffer, FsSize_t size) { + memset(buffer, 0, size); + auto header = (FsHeader*) buffer; header->version = FileStore::version(); header->size = size; + header->rootInode = sizeof(FsHeader); + + auto inodeSection = (Inode*) (buffer + header->rootInode); + inodeSection->m_id = 0; + inodeSection->next = inodeSection->prev = (uint8_t*) inodeSection - (uint8_t*) buffer; + return (uint8_t*) header; } diff --git a/test/filestoreio.cpp b/test/filestoreio.cpp index 0bd6dbb65..1c85d71c6 100644 --- a/test/filestoreio.cpp +++ b/test/filestoreio.cpp @@ -17,15 +17,25 @@ int main() { uint32_t err; FileStore32::format(volume, size); FileStore32 fs(volume, volume + size, &err); + uint32_t outSize; - fs.write(42, (void*) "Hello", 6); - - err = fs.read(42, (char*) out, nullptr); - if (err) { - return err; + if (fs.write(1, (void*) "Hello", 6) || + fs.read(1, (char*) out, &outSize) || + strcmp("Hello", out)) { + return 1; } - err = strcmp("Hello", out); + if (fs.write(2, (void*) "World", 6) || + fs.read(2, (char*) out, nullptr) || + strcmp("World", out)) { + return 1; + } - return err; + if (fs.read(1, (char*) out, &outSize) || + strcmp("Hello", out)) { + return 1; + } + + + return 0; }