From 819108098687970edb661d3fbfaac1c92ca952a0 Mon Sep 17 00:00:00 2001 From: Adrian Perez Date: Fri, 18 Jan 2019 16:43:39 -0800 Subject: [PATCH 1/5] Adapt MemoryIOSystem to delegate unhandled calls to shadowed IO system --- code/Importer.cpp | 2 +- include/assimp/MemoryIOWrapper.h | 37 +++++++++++++++++++++----------- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/code/Importer.cpp b/code/Importer.cpp index fe6df187c..da395edb2 100644 --- a/code/Importer.cpp +++ b/code/Importer.cpp @@ -483,7 +483,7 @@ const aiScene* Importer::ReadFileFromMemory( const void* pBuffer, IOSystem* io = pimpl->mIOHandler; pimpl->mIOHandler = NULL; - SetIOHandler(new MemoryIOSystem((const uint8_t*)pBuffer,pLength)); + SetIOHandler(new MemoryIOSystem((const uint8_t*)pBuffer,pLength,io)); // read the file and recover the previous IOSystem static const size_t BufSize(Importer::MaxLenHint + 28); diff --git a/include/assimp/MemoryIOWrapper.h b/include/assimp/MemoryIOWrapper.h index 5260cb2b2..540f5f32c 100644 --- a/include/assimp/MemoryIOWrapper.h +++ b/include/assimp/MemoryIOWrapper.h @@ -151,9 +151,8 @@ class MemoryIOSystem : public IOSystem { public: /** Constructor. */ - MemoryIOSystem (const uint8_t* buff, size_t len) - : buffer (buff), length(len) { - } + MemoryIOSystem(const uint8_t* buff, size_t len, IOSystem* io) + : buffer(buff), length(len), existing_io(io), created_stream(NULL) {} /** Destructor. */ ~MemoryIOSystem() { @@ -161,40 +160,52 @@ public: // ------------------------------------------------------------------- /** Tests for the existence of a file at the given path. */ - bool Exists( const char* pFile) const { - return !strncmp(pFile,AI_MEMORYIO_MAGIC_FILENAME,AI_MEMORYIO_MAGIC_FILENAME_LENGTH); + bool Exists(const char* pFile) const { + if (!strncmp(pFile,AI_MEMORYIO_MAGIC_FILENAME,AI_MEMORYIO_MAGIC_FILENAME_LENGTH)) { + return true; + } + return existing_io ? existing_io->Exists(pFile) : false; } // ------------------------------------------------------------------- /** Returns the directory separator. */ char getOsSeparator() const { - return '/'; // why not? it doesn't care + return existing_io ? existing_io->getOsSeparator() + : '/'; // why not? it doesn't care } // ------------------------------------------------------------------- /** Open a new file with a given path. */ - IOStream* Open( const char* pFile, const char* /*pMode*/ = "rb") { - if (strncmp(pFile,AI_MEMORYIO_MAGIC_FILENAME,AI_MEMORYIO_MAGIC_FILENAME_LENGTH)) { - return NULL; + IOStream* Open(const char* pFile, const char* pMode = "rb") { + if (!strncmp(pFile,AI_MEMORYIO_MAGIC_FILENAME,AI_MEMORYIO_MAGIC_FILENAME_LENGTH)) { + created_stream = new MemoryIOStream(buffer, length); + return created_stream; } - return new MemoryIOStream(buffer,length); + return existing_io ? existing_io->Open(pFile, pMode) : NULL; } // ------------------------------------------------------------------- /** Closes the given file and releases all resources associated with it. */ void Close( IOStream* pFile) { - delete pFile; + if (pFile == created_stream) { + delete pFile; + created_stream = NULL; + } else if (existing_io) { + existing_io->Close(pFile); + } } // ------------------------------------------------------------------- /** Compare two paths */ - bool ComparePaths (const char* /*one*/, const char* /*second*/) const { - return false; + bool ComparePaths(const char* one, const char* second) const { + return existing_io ? existing_io->ComparePaths(one, second) : false; } private: const uint8_t* buffer; size_t length; + IOSystem* existing_io; + IOStream* created_stream; }; } // end namespace Assimp From 87112eefaee620a7473cbcf54361a2f8dcabe5f7 Mon Sep 17 00:00:00 2001 From: Adrian Perez Date: Mon, 21 Jan 2019 14:37:33 -0800 Subject: [PATCH 2/5] Fill in rest of interface; switch created_stream to a unique_ptr --- include/assimp/MemoryIOWrapper.h | 54 ++++++++++++++++++++++++-------- 1 file changed, 41 insertions(+), 13 deletions(-) diff --git a/include/assimp/MemoryIOWrapper.h b/include/assimp/MemoryIOWrapper.h index 540f5f32c..33be00393 100644 --- a/include/assimp/MemoryIOWrapper.h +++ b/include/assimp/MemoryIOWrapper.h @@ -152,7 +152,7 @@ class MemoryIOSystem : public IOSystem public: /** Constructor. */ MemoryIOSystem(const uint8_t* buff, size_t len, IOSystem* io) - : buffer(buff), length(len), existing_io(io), created_stream(NULL) {} + : buffer(buff), length(len), existing_io(io), created_stream() {} /** Destructor. */ ~MemoryIOSystem() { @@ -160,7 +160,7 @@ public: // ------------------------------------------------------------------- /** Tests for the existence of a file at the given path. */ - bool Exists(const char* pFile) const { + bool Exists(const char* pFile) const override { if (!strncmp(pFile,AI_MEMORYIO_MAGIC_FILENAME,AI_MEMORYIO_MAGIC_FILENAME_LENGTH)) { return true; } @@ -169,27 +169,26 @@ public: // ------------------------------------------------------------------- /** Returns the directory separator. */ - char getOsSeparator() const { + char getOsSeparator() const override { return existing_io ? existing_io->getOsSeparator() - : '/'; // why not? it doesn't care + : '/'; // why not? it doesn't care } // ------------------------------------------------------------------- /** Open a new file with a given path. */ - IOStream* Open(const char* pFile, const char* pMode = "rb") { + IOStream* Open(const char* pFile, const char* pMode = "rb") override { if (!strncmp(pFile,AI_MEMORYIO_MAGIC_FILENAME,AI_MEMORYIO_MAGIC_FILENAME_LENGTH)) { - created_stream = new MemoryIOStream(buffer, length); - return created_stream; + created_stream.reset(new MemoryIOStream(buffer, length)); + return created_stream.get(); } return existing_io ? existing_io->Open(pFile, pMode) : NULL; } // ------------------------------------------------------------------- /** Closes the given file and releases all resources associated with it. */ - void Close( IOStream* pFile) { - if (pFile == created_stream) { - delete pFile; - created_stream = NULL; + void Close( IOStream* pFile) override { + if (pFile == created_stream.get()) { + created_stream.reset(); } else if (existing_io) { existing_io->Close(pFile); } @@ -197,15 +196,44 @@ public: // ------------------------------------------------------------------- /** Compare two paths */ - bool ComparePaths(const char* one, const char* second) const { + bool ComparePaths(const char* one, const char* second) const override { return existing_io ? existing_io->ComparePaths(one, second) : false; } + bool PushDirectory( const std::string &path ) override { + return existing_io ? existing_io->PushDirectory(path) : false; + } + + const std::string &CurrentDirectory() const override { + static std::string empty; + return existing_io ? existing_io->CurrentDirectory() : empty; + } + + size_t StackSize() const override { + return existing_io ? existing_io->StackSize() : 0; + } + + bool PopDirectory() override { + return existing_io ? existing_io->PopDirectory() : false; + } + + bool CreateDirectory( const std::string &path ) override { + return existing_io ? existing_io->CreateDirectory(path) : false; + } + + bool ChangeDirectory( const std::string &path ) override { + return existing_io ? existing_io->ChangeDirectory(path) : false; + } + + bool DeleteFile( const std::string &file ) override { + return existing_io ? existing_io->DeleteFile(file) : false; + } + private: const uint8_t* buffer; size_t length; IOSystem* existing_io; - IOStream* created_stream; + std::unique_ptr created_stream; }; } // end namespace Assimp From ad18cd96609cefb487980b0beed63ec72b36765d Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Tue, 22 Jan 2019 11:13:26 +0100 Subject: [PATCH 3/5] Update MemoryIOWrapper.h Fix leak. --- include/assimp/MemoryIOWrapper.h | 50 +++++++++++++++++--------------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/include/assimp/MemoryIOWrapper.h b/include/assimp/MemoryIOWrapper.h index 33be00393..4ac9d1c70 100644 --- a/include/assimp/MemoryIOWrapper.h +++ b/include/assimp/MemoryIOWrapper.h @@ -57,20 +57,16 @@ namespace Assimp { // ---------------------------------------------------------------------------------- /** Implementation of IOStream to read directly from a memory buffer */ // ---------------------------------------------------------------------------------- -class MemoryIOStream : public IOStream -{ - //friend class MemoryIOSystem; +class MemoryIOStream : public IOStream { public: MemoryIOStream (const uint8_t* buff, size_t len, bool own = false) - : buffer (buff) - , length(len) - , pos((size_t)0) - , own(own) - { + : buffer (buff) + , length(len) + , pos((size_t)0) + , own(own) { + // empty } -public: - ~MemoryIOStream () { if(own) { delete[] buffer; @@ -80,8 +76,8 @@ public: // ------------------------------------------------------------------- // Read from stream size_t Read(void* pvBuffer, size_t pSize, size_t pCount) { - ai_assert(pvBuffer); - ai_assert(pSize); + ai_assert(nullptr != pvBuffer); + ai_assert(0 != pSize); const size_t cnt = std::min(pCount,(length-pos)/pSize), ofs = pSize*cnt; memcpy(pvBuffer,buffer+pos,ofs); @@ -105,14 +101,12 @@ public: return AI_FAILURE; } pos = pOffset; - } - else if (aiOrigin_END == pOrigin) { + } else if (aiOrigin_END == pOrigin) { if (pOffset > length) { return AI_FAILURE; } pos = length-pOffset; - } - else { + } else { if (pOffset+pos > length) { return AI_FAILURE; } @@ -147,15 +141,21 @@ private: // --------------------------------------------------------------------------- /** Dummy IO system to read from a memory buffer */ -class MemoryIOSystem : public IOSystem -{ +class MemoryIOSystem : public IOSystem { public: /** Constructor. */ MemoryIOSystem(const uint8_t* buff, size_t len, IOSystem* io) - : buffer(buff), length(len), existing_io(io), created_stream() {} + : buffer(buff) + , length(len) + , existing_io(io) + , created_stream() { + // empty + } /** Destructor. */ ~MemoryIOSystem() { + delete created_stream; + created_stream = nullptr; } // ------------------------------------------------------------------- @@ -178,8 +178,8 @@ public: /** Open a new file with a given path. */ IOStream* Open(const char* pFile, const char* pMode = "rb") override { if (!strncmp(pFile,AI_MEMORYIO_MAGIC_FILENAME,AI_MEMORYIO_MAGIC_FILENAME_LENGTH)) { - created_stream.reset(new MemoryIOStream(buffer, length)); - return created_stream.get(); + created_stream = new MemoryIOStream(buffer, length); + return created_stream; } return existing_io ? existing_io->Open(pFile, pMode) : NULL; } @@ -187,8 +187,9 @@ public: // ------------------------------------------------------------------- /** Closes the given file and releases all resources associated with it. */ void Close( IOStream* pFile) override { - if (pFile == created_stream.get()) { - created_stream.reset(); + if (pFile == created_stream) { + delete created_stream; + created_stream = nullptr; } else if (existing_io) { existing_io->Close(pFile); } @@ -233,8 +234,9 @@ private: const uint8_t* buffer; size_t length; IOSystem* existing_io; - std::unique_ptr created_stream; + IOStream *created_stream; }; + } // end namespace Assimp #endif From f8a23e128bfbd9fe300005a7338952cfbf9a9ceb Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Tue, 22 Jan 2019 20:47:24 +0100 Subject: [PATCH 4/5] Update MemoryIOWrapper.h Make code more readable. --- include/assimp/MemoryIOWrapper.h | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/include/assimp/MemoryIOWrapper.h b/include/assimp/MemoryIOWrapper.h index 4ac9d1c70..7b0ea0dbc 100644 --- a/include/assimp/MemoryIOWrapper.h +++ b/include/assimp/MemoryIOWrapper.h @@ -51,6 +51,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include namespace Assimp { + #define AI_MEMORYIO_MAGIC_FILENAME "$$$___magic___$$$" #define AI_MEMORYIO_MAGIC_FILENAME_LENGTH 17 @@ -78,9 +79,11 @@ public: size_t Read(void* pvBuffer, size_t pSize, size_t pCount) { ai_assert(nullptr != pvBuffer); ai_assert(0 != pSize); - const size_t cnt = std::min(pCount,(length-pos)/pSize), ofs = pSize*cnt; + + const size_t cnt = std::min( pCount, (length-pos) / pSize); + const size_t ofs = pSize * cnt; - memcpy(pvBuffer,buffer+pos,ofs); + ::memcpy(pvBuffer,buffer+pos,ofs); pos += ofs; return cnt; @@ -161,7 +164,7 @@ public: // ------------------------------------------------------------------- /** Tests for the existence of a file at the given path. */ bool Exists(const char* pFile) const override { - if (!strncmp(pFile,AI_MEMORYIO_MAGIC_FILENAME,AI_MEMORYIO_MAGIC_FILENAME_LENGTH)) { + if (0 == strncmp( pFile, AI_MEMORYIO_MAGIC_FILENAME, AI_MEMORYIO_MAGIC_FILENAME_LENGTH ) ) { return true; } return existing_io ? existing_io->Exists(pFile) : false; @@ -177,7 +180,7 @@ public: // ------------------------------------------------------------------- /** Open a new file with a given path. */ IOStream* Open(const char* pFile, const char* pMode = "rb") override { - if (!strncmp(pFile,AI_MEMORYIO_MAGIC_FILENAME,AI_MEMORYIO_MAGIC_FILENAME_LENGTH)) { + if ( 0 == strncmp( pFile, AI_MEMORYIO_MAGIC_FILENAME, AI_MEMORYIO_MAGIC_FILENAME_LENGTH ) ) { created_stream = new MemoryIOStream(buffer, length); return created_stream; } From b04ed672886c672a45d12bbd900ba5d2b9694314 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Tue, 22 Jan 2019 20:47:24 +0100 Subject: [PATCH 5/5] Update MemoryIOWrapper.h Make code more readable. --- include/assimp/MemoryIOWrapper.h | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/include/assimp/MemoryIOWrapper.h b/include/assimp/MemoryIOWrapper.h index 4ac9d1c70..a9a78c319 100644 --- a/include/assimp/MemoryIOWrapper.h +++ b/include/assimp/MemoryIOWrapper.h @@ -51,6 +51,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include namespace Assimp { + #define AI_MEMORYIO_MAGIC_FILENAME "$$$___magic___$$$" #define AI_MEMORYIO_MAGIC_FILENAME_LENGTH 17 @@ -78,9 +79,11 @@ public: size_t Read(void* pvBuffer, size_t pSize, size_t pCount) { ai_assert(nullptr != pvBuffer); ai_assert(0 != pSize); - const size_t cnt = std::min(pCount,(length-pos)/pSize), ofs = pSize*cnt; + + const size_t cnt = std::min( pCount, (length-pos) / pSize); + const size_t ofs = pSize * cnt; - memcpy(pvBuffer,buffer+pos,ofs); + ::memcpy(pvBuffer,buffer+pos,ofs); pos += ofs; return cnt; @@ -148,20 +151,18 @@ public: : buffer(buff) , length(len) , existing_io(io) - , created_stream() { + , created_streams() { // empty } /** Destructor. */ ~MemoryIOSystem() { - delete created_stream; - created_stream = nullptr; } // ------------------------------------------------------------------- /** Tests for the existence of a file at the given path. */ bool Exists(const char* pFile) const override { - if (!strncmp(pFile,AI_MEMORYIO_MAGIC_FILENAME,AI_MEMORYIO_MAGIC_FILENAME_LENGTH)) { + if (0 == strncmp( pFile, AI_MEMORYIO_MAGIC_FILENAME, AI_MEMORYIO_MAGIC_FILENAME_LENGTH ) ) { return true; } return existing_io ? existing_io->Exists(pFile) : false; @@ -177,9 +178,9 @@ public: // ------------------------------------------------------------------- /** Open a new file with a given path. */ IOStream* Open(const char* pFile, const char* pMode = "rb") override { - if (!strncmp(pFile,AI_MEMORYIO_MAGIC_FILENAME,AI_MEMORYIO_MAGIC_FILENAME_LENGTH)) { - created_stream = new MemoryIOStream(buffer, length); - return created_stream; + if ( 0 == strncmp( pFile, AI_MEMORYIO_MAGIC_FILENAME, AI_MEMORYIO_MAGIC_FILENAME_LENGTH ) ) { + created_streams.emplace_back(new MemoryIOStream(buffer, length)); + return created_streams.back(); } return existing_io ? existing_io->Open(pFile, pMode) : NULL; } @@ -187,9 +188,10 @@ public: // ------------------------------------------------------------------- /** Closes the given file and releases all resources associated with it. */ void Close( IOStream* pFile) override { - if (pFile == created_stream) { - delete created_stream; - created_stream = nullptr; + auto it = std::find(created_streams.begin(), created_streams.end(), pFile); + if (it != created_streams.end()) { + delete pFile; + created_streams.erase(it); } else if (existing_io) { existing_io->Close(pFile); } @@ -234,7 +236,7 @@ private: const uint8_t* buffer; size_t length; IOSystem* existing_io; - IOStream *created_stream; + std::vector created_streams; }; } // end namespace Assimp