From 3a0f72424c263d1f347b4175bbe8033ce87d437a Mon Sep 17 00:00:00 2001 From: Aaron Giles Date: Thu, 24 Jun 2021 06:05:16 -0700 Subject: tools/imgtool: Removed legacy object pool usage. (#8215) --- src/tools/imgtool/imgtool.cpp | 75 +++++++++++---------------------------- src/tools/imgtool/imgtool.h | 8 ++--- src/tools/imgtool/library.cpp | 69 ++++++++--------------------------- src/tools/imgtool/library.h | 65 +++++++++++++++------------------ src/tools/imgtool/main.cpp | 6 ++-- src/tools/imgtool/modules.cpp | 2 +- src/tools/imgtool/modules/os9.cpp | 7 ++-- 7 files changed, 73 insertions(+), 159 deletions(-) diff --git a/src/tools/imgtool/imgtool.cpp b/src/tools/imgtool/imgtool.cpp index 4eaf97a1451..f16120c3d22 100644 --- a/src/tools/imgtool/imgtool.cpp +++ b/src/tools/imgtool/imgtool.cpp @@ -16,7 +16,6 @@ #include "formats/imageutl.h" #include "library.h" #include "modules.h" -#include "pool.h" /*************************************************************************** @@ -335,7 +334,7 @@ imgtoolerr_t imgtool::image::identify_file(const char *fname, imgtool_module **m /* iterate through all modules */ for (const auto &module : library.modules()) { - if (!extension || image_find_extension(module->extensions, extension)) + if (!extension || image_find_extension(module->extensions.c_str(), extension)) { err = evaluate_module(fname, module.get(), val); if (err) @@ -533,16 +532,6 @@ imgtoolerr_t imgtool::image::list_partitions(std::vectorname) + if (module->name.empty()) { - util::stream_format(std::wcerr, L"imgtool module %s has null 'name'\n", wstring_from_utf8(module->name)); + util::stream_format(std::wcerr, L"imgtool module has null 'name'\n"); error = true; } - if (!module->description) + if (module->description.empty()) { util::stream_format(std::wcerr, L"imgtool module %s has null 'description'\n", wstring_from_utf8(module->name)); error = true; } - if (!module->extensions) + if (module->extensions.empty()) { util::stream_format(std::wcerr, L"imgtool module %s has null 'extensions'\n", wstring_from_utf8(module->extensions)); error = true; @@ -894,24 +883,24 @@ bool imgtool_validitychecks(void) #endif /* sanity checks on creation options */ - if (module->createimage_optguide || module->createimage_optspec) + if (module->createimage_optguide || !module->createimage_optspec.empty()) { if (!module->create) { util::stream_format(std::wcerr, L"imgtool module %s has creation options without supporting create\n", wstring_from_utf8(module->name)); error = true; } - if ((!module->createimage_optguide && module->createimage_optspec) - || (module->createimage_optguide && !module->createimage_optspec)) + if ((!module->createimage_optguide && !module->createimage_optspec.empty()) + || (module->createimage_optguide && module->createimage_optspec.empty())) { util::stream_format(std::wcerr, L"imgtool module %s does has partially incomplete creation options\n", wstring_from_utf8(module->name)); error = true; } - if (module->createimage_optguide && module->createimage_optspec) + if (module->createimage_optguide && !module->createimage_optspec.empty()) { auto resolution = std::make_unique(*module->createimage_optguide); - resolution->set_specification(module->createimage_optspec); + resolution->set_specification(module->createimage_optspec.c_str()); } } } @@ -954,8 +943,6 @@ imgtoolerr_t imgtool::image::internal_open(const imgtool_module *module, const s imgtoolerr_t err; imgtool::stream::ptr stream; imgtool::image::ptr image; - object_pool *pool = nullptr; - void *extra_bytes = nullptr; outimg.reset(); @@ -966,14 +953,6 @@ imgtoolerr_t imgtool::image::internal_open(const imgtool_module *module, const s goto done; } - // create a memory pool - pool = pool_alloc_lib(nullptr); - if (!pool) - { - err = imgtoolerr_t(IMGTOOLERR_OUTOFMEMORY); - goto done; - } - // open the stream stream = imgtool::stream::open(filename, read_or_write); if (!stream) @@ -982,29 +961,14 @@ imgtoolerr_t imgtool::image::internal_open(const imgtool_module *module, const s goto done; } - // allocate extra - if (module->image_extra_bytes > 0) - { - extra_bytes = pool_malloc_lib(pool, module->image_extra_bytes); - if (!extra_bytes) - { - err = imgtoolerr_t(IMGTOOLERR_OUTOFMEMORY); - goto done; - } - memset(extra_bytes, 0, module->image_extra_bytes); - } - // setup the image structure - try { image = std::make_unique(*module, pool, extra_bytes); } + try { image = std::make_unique(*module); } catch (std::bad_alloc const &) { err = imgtoolerr_t(IMGTOOLERR_OUTOFMEMORY); goto done; } - // the pool is no longer owned by this function - pool = nullptr; - // actually call create or open err = (read_or_write == OSD_FOPEN_RW_CREATE) ? module->create(*image, std::move(stream), createopts) @@ -1021,8 +985,6 @@ imgtoolerr_t imgtool::image::internal_open(const imgtool_module *module, const s outimg = std::move(image); done: - if (pool) - pool_free_lib(pool); return err; } @@ -1058,12 +1020,16 @@ imgtoolerr_t imgtool::image::open(const std::string &modulename, const std::stri // imgtool::image::image //------------------------------------------------- -imgtool::image::image(const imgtool_module &module, object_pool *pool, void *extra_bytes) +imgtool::image::image(const imgtool_module &module) : m_module(module) - , m_pool(pool) - , m_extra_bytes(extra_bytes) , m_okay_to_close(false) { + if (module.image_extra_bytes > 0) + { + m_extra_bytes = std::make_unique(module.image_extra_bytes); + std::fill_n(&m_extra_bytes[0], module.image_extra_bytes, 0); + } + } @@ -1075,7 +1041,6 @@ imgtool::image::~image() { if (m_okay_to_close && module().close) module().close(*this); - pool_free_lib(m_pool); } @@ -1094,8 +1059,8 @@ imgtoolerr_t imgtool::image::create(const imgtool_module *module, const std::str try { alloc_resolution.reset(new util::option_resolution(*module->createimage_optguide)); } catch (...) { return (imgtoolerr_t)IMGTOOLERR_OUTOFMEMORY; } - if (module->createimage_optspec) - alloc_resolution->set_specification(module->createimage_optspec); + if (!module->createimage_optspec.empty()) + alloc_resolution->set_specification(module->createimage_optspec.c_str()); opts = alloc_resolution.get(); } diff --git a/src/tools/imgtool/imgtool.h b/src/tools/imgtool/imgtool.h index ca8d19aed85..c7b59aafeee 100644 --- a/src/tools/imgtool/imgtool.h +++ b/src/tools/imgtool/imgtool.h @@ -95,7 +95,7 @@ namespace imgtool public: typedef std::unique_ptr ptr; - image(const imgtool_module &module, object_pool *pool, void *extra_bytes); + image(const imgtool_module &module); ~image(); static imgtoolerr_t identify_file(const char *filename, imgtool_module **modules, size_t count); @@ -115,14 +115,12 @@ namespace imgtool imgtoolerr_t write_block(uint64_t block, const void *buffer); imgtoolerr_t clear_block(uint64_t block, uint8_t data); imgtoolerr_t list_partitions(std::vector &partitions); - void *malloc(size_t size); const imgtool_module &module() { return m_module; } - void *extra_bytes() { return m_extra_bytes; } + void *extra_bytes() { return m_extra_bytes.get(); } private: const imgtool_module &m_module; - object_pool *m_pool; - void *m_extra_bytes; + std::unique_ptr m_extra_bytes; // because of an idiosyncrasy of how imgtool::image::internal_open() works, we are only "okay to close" // by invoking the module's close function once internal_open() succeeds. the long term solution is diff --git a/src/tools/imgtool/library.cpp b/src/tools/imgtool/library.cpp index e3c75203805..48d440ec701 100644 --- a/src/tools/imgtool/library.cpp +++ b/src/tools/imgtool/library.cpp @@ -14,7 +14,6 @@ #include "imgtool.h" #include "library.h" -#include "pool.h" namespace imgtool { @@ -145,9 +144,6 @@ time_t datetime::to_time_t() const library::library() { - m_pool = pool_alloc_lib(nullptr); - if (!m_pool) - throw std::bad_alloc(); } @@ -157,7 +153,6 @@ library::library() library::~library() { - pool_free_lib(m_pool); } @@ -167,28 +162,23 @@ library::~library() void library::add_class(const imgtool_class *imgclass) { - char *s1, *s2; + char const *temp; // allocate the module and place it in the chain m_modules.emplace_back(std::make_unique()); imgtool_module *module = m_modules.back().get(); - memset(module, 0, sizeof(*module)); - - // extensions have a weird format - s1 = imgtool_get_info_string(imgclass, IMGTOOLINFO_STR_FILE_EXTENSIONS); - s2 = (char*)imgtool_library_malloc(strlen(s1) + 1); - strcpy(s2, s1); - module->extensions = s2; module->imgclass = *imgclass; - module->name = imgtool_library_strdup(imgtool_get_info_string(imgclass, IMGTOOLINFO_STR_NAME)); - module->description = imgtool_library_strdup(imgtool_get_info_string(imgclass, IMGTOOLINFO_STR_DESCRIPTION)); - module->eoln = imgtool_library_strdup_allow_null(imgtool_get_info_string(imgclass, IMGTOOLINFO_STR_EOLN)); - module->initial_path_separator = imgtool_get_info_int(imgclass, IMGTOOLINFO_INT_INITIAL_PATH_SEPARATOR) ? 1 : 0; - module->open_is_strict = imgtool_get_info_int(imgclass, IMGTOOLINFO_INT_OPEN_IS_STRICT) ? 1 : 0; - module->tracks_are_called_cylinders = imgtool_get_info_int(imgclass, IMGTOOLINFO_INT_TRACKS_ARE_CALLED_CYLINDERS) ? 1 : 0; - module->writing_untested = imgtool_get_info_int(imgclass, IMGTOOLINFO_INT_WRITING_UNTESTED) ? 1 : 0; - module->creation_untested = imgtool_get_info_int(imgclass, IMGTOOLINFO_INT_CREATION_UNTESTED) ? 1 : 0; + module->name = imgtool_get_info_string(imgclass, IMGTOOLINFO_STR_NAME); + module->description = imgtool_get_info_string(imgclass, IMGTOOLINFO_STR_DESCRIPTION); + module->extensions = imgtool_get_info_string(imgclass, IMGTOOLINFO_STR_FILE_EXTENSIONS); + temp = imgtool_get_info_string(imgclass, IMGTOOLINFO_STR_EOLN); + module->eoln = (temp != nullptr) ? temp : ""; + module->initial_path_separator = imgtool_get_info_int(imgclass, IMGTOOLINFO_INT_INITIAL_PATH_SEPARATOR) ? true : false; + module->open_is_strict = imgtool_get_info_int(imgclass, IMGTOOLINFO_INT_OPEN_IS_STRICT) ? true : false; + module->tracks_are_called_cylinders = imgtool_get_info_int(imgclass, IMGTOOLINFO_INT_TRACKS_ARE_CALLED_CYLINDERS) ? true : false; + module->writing_untested = imgtool_get_info_int(imgclass, IMGTOOLINFO_INT_WRITING_UNTESTED) ? true : false; + module->creation_untested = imgtool_get_info_int(imgclass, IMGTOOLINFO_INT_CREATION_UNTESTED) ? true : false; module->open = (imgtoolerr_t (*)(imgtool::image &, imgtool::stream::ptr &&)) imgtool_get_info_fct(imgclass, IMGTOOLINFO_PTR_OPEN); module->create = (imgtoolerr_t (*)(imgtool::image &, imgtool::stream::ptr &&, util::option_resolution *)) imgtool_get_info_fct(imgclass, IMGTOOLINFO_PTR_CREATE); module->close = (void (*)(imgtool::image &)) imgtool_get_info_fct(imgclass, IMGTOOLINFO_PTR_CLOSE); @@ -201,7 +191,8 @@ void library::add_class(const imgtool_class *imgclass) module->list_partitions = (imgtoolerr_t (*)(imgtool::image &, std::vector &)) imgtool_get_info_fct(imgclass, IMGTOOLINFO_PTR_LIST_PARTITIONS); module->block_size = imgtool_get_info_int(imgclass, IMGTOOLINFO_INT_BLOCK_SIZE); module->createimage_optguide = (const util::option_guide *) imgtool_get_info_ptr(imgclass, IMGTOOLINFO_PTR_CREATEIMAGE_OPTGUIDE); - module->createimage_optspec = imgtool_library_strdup_allow_null((const char*)imgtool_get_info_ptr(imgclass, IMGTOOLINFO_STR_CREATEIMAGE_OPTSPEC)); + temp = (char const *)imgtool_get_info_ptr(imgclass, IMGTOOLINFO_STR_CREATEIMAGE_OPTSPEC); + module->createimage_optspec = (temp != nullptr) ? temp : ""; module->image_extra_bytes += imgtool_get_info_int(imgclass, IMGTOOLINFO_INT_IMAGE_EXTRA_BYTES); } @@ -269,10 +260,10 @@ int library::module_compare(const imgtool_module *m1, const imgtool_module *m2, switch(sort) { case sort_type::NAME: - rc = strcmp(m1->name, m2->name); + rc = strcmp(m1->name.c_str(), m2->name.c_str()); break; case sort_type::DESCRIPTION: - rc = core_stricmp(m1->description, m2->description); + rc = core_stricmp(m1->description.c_str(), m2->description.c_str()); break; } return rc; @@ -319,34 +310,4 @@ const imgtool_module *library::findmodule(const std::string &module_name) } -//------------------------------------------------- -// imgtool_library_malloc -//------------------------------------------------- - -void *library::imgtool_library_malloc(size_t mem) -{ - return pool_malloc_lib(m_pool, mem); -} - - -//------------------------------------------------- -// imgtool_library_malloc -//------------------------------------------------- - -char *library::imgtool_library_strdup(const char *s) -{ - return pool_strdup_lib(m_pool, s); -} - - -//------------------------------------------------- -// imgtool_library_strdup_allow_null -//------------------------------------------------- - -char *library::imgtool_library_strdup_allow_null(const char *s) -{ - return s ? imgtool_library_strdup(s) : nullptr; -} - - } // namespace imgtool diff --git a/src/tools/imgtool/library.h b/src/tools/imgtool/library.h index 6dc9ea8224b..7406cc2e526 100644 --- a/src/tools/imgtool/library.h +++ b/src/tools/imgtool/library.h @@ -26,7 +26,6 @@ #include "stream.h" #include "unicode.h" #include "charconv.h" -#include "pool.h" #include "timeconv.h" namespace imgtool @@ -459,39 +458,39 @@ char *imgtool_temp_str(void); struct imgtool_module { - imgtool_class imgclass; + imgtool_class imgclass = { 0 }; - const char *name; - const char *description; - const char *extensions; - const char *eoln; + std::string name; + std::string description; + std::string extensions; + std::string eoln; - size_t image_extra_bytes; + size_t image_extra_bytes = 0; /* flags */ - unsigned int initial_path_separator : 1; - unsigned int open_is_strict : 1; - unsigned int tracks_are_called_cylinders : 1; /* used for hard drivers */ - unsigned int writing_untested : 1; /* used when we support writing, but not in main build */ - unsigned int creation_untested : 1; /* used when we support creation, but not in main build */ - - imgtoolerr_t (*open) (imgtool::image &image, imgtool::stream::ptr &&stream); - void (*close) (imgtool::image &image); - void (*info) (imgtool::image &image, std::ostream &stream); - imgtoolerr_t (*create) (imgtool::image &image, imgtool::stream::ptr &&stream, util::option_resolution *opts); - imgtoolerr_t (*get_geometry) (imgtool::image &image, uint32_t *track, uint32_t *heads, uint32_t *sectors); - imgtoolerr_t (*read_sector) (imgtool::image &image, uint32_t track, uint32_t head, uint32_t sector, std::vector &buffer); - imgtoolerr_t (*write_sector) (imgtool::image &image, uint32_t track, uint32_t head, uint32_t sector, const void *buffer, size_t len); - imgtoolerr_t (*read_block) (imgtool::image &image, void *buffer, uint64_t block); - imgtoolerr_t (*write_block) (imgtool::image &image, const void *buffer, uint64_t block); - imgtoolerr_t (*list_partitions)(imgtool::image &image, std::vector &partitions); - - uint32_t block_size; - - const util::option_guide *createimage_optguide; - const char *createimage_optspec; - - const void *extra; + bool initial_path_separator = false; + bool open_is_strict = false; + bool tracks_are_called_cylinders = false; /* used for hard drivers */ + bool writing_untested = false; /* used when we support writing, but not in main build */ + bool creation_untested = false; /* used when we support creation, but not in main build */ + + imgtoolerr_t (*open) (imgtool::image &image, imgtool::stream::ptr &&stream) = nullptr; + void (*close) (imgtool::image &image) = nullptr; + void (*info) (imgtool::image &image, std::ostream &stream) = nullptr; + imgtoolerr_t (*create) (imgtool::image &image, imgtool::stream::ptr &&stream, util::option_resolution *opts) = nullptr; + imgtoolerr_t (*get_geometry) (imgtool::image &image, uint32_t *track, uint32_t *heads, uint32_t *sectors) = nullptr; + imgtoolerr_t (*read_sector) (imgtool::image &image, uint32_t track, uint32_t head, uint32_t sector, std::vector &buffer) = nullptr; + imgtoolerr_t (*write_sector) (imgtool::image &image, uint32_t track, uint32_t head, uint32_t sector, const void *buffer, size_t len) = nullptr; + imgtoolerr_t (*read_block) (imgtool::image &image, void *buffer, uint64_t block) = nullptr; + imgtoolerr_t (*write_block) (imgtool::image &image, const void *buffer, uint64_t block) = nullptr; + imgtoolerr_t (*list_partitions)(imgtool::image &image, std::vector &partitions) = nullptr; + + uint32_t block_size = 0; + + const util::option_guide *createimage_optguide = nullptr; + std::string createimage_optspec; + + const void *extra = nullptr; }; namespace imgtool { @@ -531,7 +530,6 @@ public: const modulelist &modules() { return m_modules; } private: - object_pool * m_pool; modulelist m_modules; // internal lookup and iteration @@ -540,11 +538,6 @@ private: // helpers void add_class(const imgtool_class *imgclass); int module_compare(const imgtool_module *m1, const imgtool_module *m2, sort_type sort); - - // memory allocators for pooled library memory (these should go away in further C++-ification) - void *imgtool_library_malloc(size_t mem); - char *imgtool_library_strdup(const char *s); - char *imgtool_library_strdup_allow_null(const char *s); }; } // namespace imgtool diff --git a/src/tools/imgtool/main.cpp b/src/tools/imgtool/main.cpp index a0a9be24054..75fcd52e7a2 100644 --- a/src/tools/imgtool/main.cpp +++ b/src/tools/imgtool/main.cpp @@ -597,7 +597,7 @@ static int cmd_create(const struct command *c, int argc, char *argv[]) goto error; } - if (module->createimage_optguide && module->createimage_optspec) + if (module->createimage_optguide && !module->createimage_optspec.empty()) { try { resolution.reset(new util::option_resolution(*module->createimage_optguide)); } catch (...) @@ -605,7 +605,7 @@ static int cmd_create(const struct command *c, int argc, char *argv[]) err = IMGTOOLERR_OUTOFMEMORY; goto error; } - resolution->set_specification(module->createimage_optspec); + resolution->set_specification(module->createimage_optspec.c_str()); } unnamedargs = parse_options(argc, argv, 2, 3, resolution.get(), nullptr, nullptr); @@ -831,7 +831,7 @@ static int cmd_listdriveroptions(const struct command *c, int argc, char *argv[] if (opt_guide) { util::stream_format(std::wcout, L"Image specific creation options (usable on the 'create' command):\n\n"); - listoptions(*opt_guide, mod->createimage_optspec); + listoptions(*opt_guide, mod->createimage_optspec.c_str()); util::stream_format(std::wcout, L"\n"); } else diff --git a/src/tools/imgtool/modules.cpp b/src/tools/imgtool/modules.cpp index a42ea6be6f0..2ccd369976a 100644 --- a/src/tools/imgtool/modules.cpp +++ b/src/tools/imgtool/modules.cpp @@ -60,7 +60,7 @@ imgtoolerr_t imgtool_create_canonical_library(bool omit_untested, std::unique_pt { module->create = nullptr; module->createimage_optguide = nullptr; - module->createimage_optspec = nullptr; + module->createimage_optspec.clear(); } } } diff --git a/src/tools/imgtool/modules/os9.cpp b/src/tools/imgtool/modules/os9.cpp index 69dfc0e6ad1..8c9f24959ce 100644 --- a/src/tools/imgtool/modules/os9.cpp +++ b/src/tools/imgtool/modules/os9.cpp @@ -44,7 +44,7 @@ struct os9_diskinfo unsigned int octal_track_density : 1; char name[33]; - uint8_t *allocation_bitmap; + uint8_t allocation_bitmap[65536]; }; @@ -670,10 +670,7 @@ static imgtoolerr_t os9_diskimage_open(imgtool::image &image, imgtool::stream::p return IMGTOOLERR_CORRUPTIMAGE; /* is the allocation bitmap too big? */ - info->allocation_bitmap = (uint8_t*)image.malloc(info->allocation_bitmap_bytes); - if (!info->allocation_bitmap) - return IMGTOOLERR_OUTOFMEMORY; - memset(info->allocation_bitmap, 0, info->allocation_bitmap_bytes); + memset(&info->allocation_bitmap[0], 0, info->allocation_bitmap_bytes); /* sectors per track and track size don't jive? */ if (info->sectors_per_track != track_size_in_sectors) -- cgit v1.2.3