From 0df8c0051982d62399f7d6f7f440667e3ebaeb50 Mon Sep 17 00:00:00 2001 From: Hermet Park Date: Mon, 21 Jun 2021 20:18:08 +0900 Subject: [PATCH] loaders: revise code from cce4b443b3250d14a3077cf13226f7cacbb82f57 use copy variable instead of additional buffer pointer. --- inc/thorvg.h | 4 ++- src/lib/tvgPicture.cpp | 2 +- src/loaders/svg/tvgSvgLoader.cpp | 26 ++++++++-------- src/loaders/svg/tvgSvgLoader.h | 5 +-- src/loaders/tvg/tvgTvgLoadParser.cpp | 2 +- src/loaders/tvg/tvgTvgLoader.cpp | 46 +++++++++++++++------------- src/loaders/tvg/tvgTvgLoader.h | 6 ++-- 7 files changed, 48 insertions(+), 43 deletions(-) diff --git a/inc/thorvg.h b/inc/thorvg.h index 7ecf6a9f..2d352543 100644 --- a/inc/thorvg.h +++ b/inc/thorvg.h @@ -989,7 +989,7 @@ public: * * @param[in] data A pointer to a memory location where the content of the picture file is stored. * @param[in] size The size in bytes of the memory occupied by the @p data. - * @param[in] copy Decides whether the data should be copied into the local buffer. + * @param[in] copy Decides whether the data should be copied into the engine local buffer. * * @retval Result::Success When succeed. * @retval Result::InvalidArguments In case no data are provided or the @p size is zero or less. @@ -997,6 +997,8 @@ public: * @retval Result::Unknown If an error occurs at a later stage. * * @note: This api supports only SVG format + * + * @warning: you have responsibility to release the @p data memory if the @p copy is true */ Result load(const char* data, uint32_t size, bool copy = false) noexcept; diff --git a/src/lib/tvgPicture.cpp b/src/lib/tvgPicture.cpp index db12e44a..6f10c6f4 100644 --- a/src/lib/tvgPicture.cpp +++ b/src/lib/tvgPicture.cpp @@ -53,7 +53,7 @@ Result Picture::load(const std::string& path) noexcept } -Result Picture::load(const char* data, uint32_t size, bool copy /*=false*/) noexcept +Result Picture::load(const char* data, uint32_t size, bool copy) noexcept { if (!data || size <= 0) return Result::InvalidArguments; diff --git a/src/loaders/svg/tvgSvgLoader.cpp b/src/loaders/svg/tvgSvgLoader.cpp index 1c6f78b5..f660e841 100644 --- a/src/loaders/svg/tvgSvgLoader.cpp +++ b/src/loaders/svg/tvgSvgLoader.cpp @@ -2563,12 +2563,12 @@ static bool _svgLoaderParserForValidCheck(void* data, SimpleXMLType type, const } -void SvgLoader::clearBuffer() +void SvgLoader::clear() { - free(buffer); + if (copy) free((char*)content); size = 0; - buffer = nullptr; content = nullptr; + copy = false; } @@ -2644,17 +2644,16 @@ bool SvgLoader::header() bool SvgLoader::open(const char* data, uint32_t size, bool copy) { - clearBuffer(); + clear(); + if (copy) { - buffer = (char*)malloc(size); - if (!buffer) return false; - memcpy(buffer, data, size); - this->content = buffer; - } else { - this->content = data; - } + content = (char*)malloc(size); + if (!content) return false; + memcpy((char*)content, data, size); + } else content = data; this->size = size; + this->copy = copy; return header(); } @@ -2662,8 +2661,7 @@ bool SvgLoader::open(const char* data, uint32_t size, bool copy) bool SvgLoader::open(const string& path) { - //TODO: verify memory leak if open() is called multiple times. - clearBuffer(); + clear(); ifstream f; f.open(path); @@ -2711,7 +2709,7 @@ bool SvgLoader::close() loaderData.doc = nullptr; loaderData.stack.reset(); - clearBuffer(); + clear(); return true; } diff --git a/src/loaders/svg/tvgSvgLoader.h b/src/loaders/svg/tvgSvgLoader.h index abddeb79..c213d66a 100644 --- a/src/loaders/svg/tvgSvgLoader.h +++ b/src/loaders/svg/tvgSvgLoader.h @@ -29,13 +29,14 @@ class SvgLoader : public Loader, public Task { public: string filePath; - char* buffer = nullptr; const char* content = nullptr; uint32_t size = 0; SvgLoaderData loaderData; unique_ptr root; + bool copy = false; + SvgLoader(); ~SvgLoader(); @@ -51,7 +52,7 @@ public: unique_ptr scene() override; private: - void clearBuffer(); + void clear(); }; diff --git a/src/loaders/tvg/tvgTvgLoadParser.cpp b/src/loaders/tvg/tvgTvgLoadParser.cpp index f8e9af76..c7400f10 100644 --- a/src/loaders/tvg/tvgTvgLoadParser.cpp +++ b/src/loaders/tvg/tvgTvgLoadParser.cpp @@ -542,4 +542,4 @@ unique_ptr tvgLoadData(const char *ptr, uint32_t size) } return move(scene); -} +} \ No newline at end of file diff --git a/src/loaders/tvg/tvgTvgLoader.cpp b/src/loaders/tvg/tvgTvgLoader.cpp index 9bab82bd..5e793ecc 100644 --- a/src/loaders/tvg/tvgTvgLoader.cpp +++ b/src/loaders/tvg/tvgTvgLoader.cpp @@ -31,12 +31,13 @@ /* Internal Class Implementation */ /************************************************************************/ -void TvgLoader::clearBuffer() +void TvgLoader::clear() { - free(buffer); - size = 0; - buffer = nullptr; + if (copy) free((char*)data); + data = nullptr; pointer = nullptr; + size = 0; + copy = false; } @@ -52,7 +53,7 @@ TvgLoader::~TvgLoader() bool TvgLoader::open(const string &path) { - //TODO: verify memory leak if open() is called multiple times. + clear(); ifstream f; f.open(path, ifstream::in | ifstream::binary | ifstream::ate); @@ -62,40 +63,41 @@ bool TvgLoader::open(const string &path) size = f.tellg(); f.seekg(0, ifstream::beg); - buffer = (char*)malloc(size); - if (!buffer) { - size = 0; + copy = true; + data = (char*)malloc(size); + if (!data) { + clear(); f.close(); return false; } - if (!f.read(buffer, size)) + if (!f.read((char*)data, size)) { - clearBuffer(); + clear(); f.close(); return false; } f.close(); - pointer = buffer; + pointer = data; return tvgValidateData(pointer, size); } bool TvgLoader::open(const char *data, uint32_t size, bool copy) { - if (copy) { - if (buffer) clearBuffer(); - buffer = (char*)malloc(size); - if (!buffer) return false; - memcpy(buffer, data, size); - this->pointer = buffer; - } else { - this->pointer = data; - } + clear(); + if (copy) { + this->data = (char*)malloc(size); + if (!this->data) return false; + memcpy((char*)this->data, data, size); + } else this->data = data; + + this->pointer = this->data; this->size = size; + this->copy = copy; return tvgValidateData(pointer, size); } @@ -112,7 +114,7 @@ bool TvgLoader::read() bool TvgLoader::close() { this->done(); - clearBuffer(); + clear(); return true; } @@ -120,7 +122,7 @@ void TvgLoader::run(unsigned tid) { if (root) root.reset(); root = tvgLoadData(pointer, size); - if (!root) clearBuffer(); + if (!root) clear(); } unique_ptr TvgLoader::scene() diff --git a/src/loaders/tvg/tvgTvgLoader.h b/src/loaders/tvg/tvgTvgLoader.h index d0aec4f8..155365df 100644 --- a/src/loaders/tvg/tvgTvgLoader.h +++ b/src/loaders/tvg/tvgTvgLoader.h @@ -28,12 +28,14 @@ class TvgLoader : public Loader, public Task { public: - char* buffer = nullptr; + const char* data = nullptr; const char* pointer = nullptr; uint32_t size = 0; unique_ptr root = nullptr; + bool copy = false; + ~TvgLoader(); using Loader::open; @@ -46,7 +48,7 @@ public: unique_ptr scene() override; private: - void clearBuffer(); + void clear(); }; #endif //_TVG_TVG_LOADER_H_