From c9c708dfbe97f6b2920b4e7f6159ae5b9d2376cd Mon Sep 17 00:00:00 2001 From: Twan van Laarhoven Date: Wed, 27 May 2020 22:16:48 +0200 Subject: [PATCH] Fix #63: don't use reference arguments in openFileInPackage --- src/script/parser.cpp | 4 ++-- src/util/io/package.cpp | 2 +- src/util/io/package_manager.cpp | 13 +++++++------ src/util/io/package_manager.hpp | 6 +++--- src/util/io/reader.cpp | 4 ---- src/util/io/reader.hpp | 6 +++--- src/util/spell_checker.cpp | 3 +-- 7 files changed, 17 insertions(+), 21 deletions(-) diff --git a/src/script/parser.cpp b/src/script/parser.cpp index 26582279..ea9a8e14 100644 --- a/src/script/parser.cpp +++ b/src/script/parser.cpp @@ -683,10 +683,10 @@ ExprType parseExpr(TokenIterator& input, Script& script, Precedence minPrec) { // include the file // read the entire file, and start at the beginning of it String const& filename = token.value; - auto stream = package_manager.openFileFromPackage(input.package, filename); + auto [stream,file_package] = package_manager.openFileFromPackage(input.package, filename); eat_utf8_bom(*stream); String included_input = read_utf8_line(*stream, true); - TokenIterator included_tokens(included_input, input.package, false, filename, input.errors); + TokenIterator included_tokens(included_input, file_package, false, filename, input.errors); return parseTopLevel(included_tokens, script); } else { // variable diff --git a/src/util/io/package.cpp b/src/util/io/package.cpp index 0d7c8894..272cfa52 100644 --- a/src/util/io/package.cpp +++ b/src/util/io/package.cpp @@ -190,7 +190,7 @@ unique_ptr Package::openIn(const String& file) { if (!file.empty() && file.GetChar(0) == _('/')) { // absolute path, open file from another package Packaged* p = dynamic_cast(this); - return package_manager.openFileFromPackage(p, file); + return package_manager.openFileFromPackage(p, file).first; } FileInfos::iterator it = files.find(normalize_internal_filename(file)); if (it == files.end()) { diff --git a/src/util/io/package_manager.cpp b/src/util/io/package_manager.cpp index 698703eb..2f368ad0 100644 --- a/src/util/io/package_manager.cpp +++ b/src/util/io/package_manager.cpp @@ -95,7 +95,7 @@ void PackageManager::findMatching(const String& pattern, vector& out) } } -unique_ptr PackageManager::openFileFromPackage(Packaged*& package, const String& name) { +pair,Packaged*> PackageManager::openFileFromPackage(Packaged* package, const String& name) { if (!name.empty() && name.GetChar(0) == _('/')) { // absolute name; break name size_t start = name.find_first_not_of(_("/\\"), 1); // allow "//package/name" from incorrect scripts @@ -106,17 +106,19 @@ unique_ptr PackageManager::openFileFromPackage(Packaged*& package if (package && !is_substr(name,start,_(":NO-WARN-DEP:"))) { package->requireDependency(p.get()); } - package = p.get(); - return p->openIn(name.substr(pos + 1)); + return {p->openIn(name.substr(pos + 1)), p.get()}; } } else if (package) { // relative name - return package->openIn(name); + return {package->openIn(name), package}; } throw FileNotFoundError(name, _("No package name specified, use '/package/filename'")); } +pair, Packaged*> openFileFromPackage(Packaged* package, const String& name) { + return package_manager.openFileFromPackage(package, name); +} -String PackageManager::openFilenameFromPackage(Packaged*& package, const String& name) { +String PackageManager::openFilenameFromPackage(Packaged* package, const String& name) { if (!name.empty() && name.GetChar(0) == _('/')) { // absolute name; break name size_t start = name.find_first_not_of(_("/\\"), 1); // allow "//package/name" from incorrect scripts @@ -127,7 +129,6 @@ String PackageManager::openFilenameFromPackage(Packaged*& package, const String& if (package && !is_substr(name,start,_(":NO-WARN-DEP:"))) { package->requireDependency(p.get()); } - package = p.get(); return p->absoluteFilename() + _("/") + name.substr(pos + 1); } } else if (package) { diff --git a/src/util/io/package_manager.hpp b/src/util/io/package_manager.hpp index 4d2a1eac..4178d310 100644 --- a/src/util/io/package_manager.hpp +++ b/src/util/io/package_manager.hpp @@ -145,15 +145,15 @@ public: * - tries to open a relative file from the package if the name is "file" * - verifies a dependency from that package if an absolute filename is used * this is to force people to fill in the dependencies - * Afterwards, package will be set to the package the file is opened from + * Returns the opened file and the package it is in */ - unique_ptr openFileFromPackage(Packaged*& package, const String& name); + pair,Packaged*> openFileFromPackage(Packaged* package, const String& name); /// Get a filename to open from a package /** WARNING: this is a bit of a hack, since not all package types support names in this way. * It is needed for third party libraries (i.e. hunspell) that load stuff from files. */ - String openFilenameFromPackage(Packaged*& package, const String& name); + String openFilenameFromPackage(Packaged* package, const String& name); // --------------------------------------------------- : Packages on disk diff --git a/src/util/io/reader.cpp b/src/util/io/reader.cpp index cd1ecc98..a641c5f9 100644 --- a/src/util/io/reader.cpp +++ b/src/util/io/reader.cpp @@ -29,10 +29,6 @@ Reader::Reader(wxInputStream& input, Packaged* package, const String& filename, handleAppVersion(); } -unique_ptr Reader::openIncludedFile() { - return package_manager.openFileFromPackage(package, value); -} - void Reader::handleIgnore(int end_version, const Char* a) { if (file_app_version < end_version) { if (enterBlock(a)) exitBlock(); diff --git a/src/util/io/reader.hpp b/src/util/io/reader.hpp index 9eaa70fe..eae1c4b3 100644 --- a/src/util/io/reader.hpp +++ b/src/util/io/reader.hpp @@ -16,6 +16,7 @@ template class Scriptable; DECLARE_POINTER_TYPE(Game); DECLARE_POINTER_TYPE(StyleSheet); class Packaged; +pair, Packaged*> openFileFromPackage(Packaged* package, const String& name); // ----------------------------------------------------------------------------- : Reader @@ -171,8 +172,8 @@ private: template void unknownKey(T& v) { if (key == _("include_file")) { - auto stream = openIncludedFile(); - Reader sub_reader(*stream, package, value, ignore_invalid); + auto [stream, include_package] = openFileFromPackage(package, value); + Reader sub_reader(*stream, include_package, value, ignore_invalid); if (sub_reader.file_app_version == 0) { // in an included file, use the app version of the parent if there is none sub_reader.file_app_version = file_app_version; @@ -184,7 +185,6 @@ private: } } void unknownKey(); - unique_ptr openIncludedFile(); }; // ----------------------------------------------------------------------------- : After reading hook diff --git a/src/util/spell_checker.cpp b/src/util/spell_checker.cpp index aa7b2bb6..50fa7bb0 100644 --- a/src/util/spell_checker.cpp +++ b/src/util/spell_checker.cpp @@ -36,8 +36,7 @@ SpellChecker* SpellChecker::get(const String& language) { SpellChecker* SpellChecker::get(const String& filename, const String& language) { SpellCheckerP& speller = spellers[filename + _(".") + language]; if (!speller) { - Packaged* package = nullptr; - String prefix = package_manager.openFilenameFromPackage(package, filename) + _("."); + String prefix = package_manager.openFilenameFromPackage(nullptr, filename) + _("."); String local_dir = package_manager.getDictionaryDir(true); String global_dir = package_manager.getDictionaryDir(false); String aff_path = language + _(".aff");