From 3901ef9760495355433e6e7d6c39655ae256c5bd Mon Sep 17 00:00:00 2001 From: Rubidium Date: Thu, 27 Apr 2023 20:42:25 +0200 Subject: [PATCH] Codechange: use std::string for the GRF filenames --- src/network/core/tcp_content.cpp | 2 +- src/newgrf.cpp | 21 ++++++++++----------- src/newgrf.h | 2 +- src/newgrf_config.cpp | 24 ++++++++---------------- src/newgrf_config.h | 4 ++-- src/newgrf_gui.cpp | 4 ++-- 6 files changed, 24 insertions(+), 33 deletions(-) diff --git a/src/network/core/tcp_content.cpp b/src/network/core/tcp_content.cpp index 14108fb8ab..dbbb70dcaa 100644 --- a/src/network/core/tcp_content.cpp +++ b/src/network/core/tcp_content.cpp @@ -71,7 +71,7 @@ const char *ContentInfo::GetTextfile(TextfileType type) const break; case CONTENT_TYPE_NEWGRF: { const GRFConfig *gc = FindGRFConfig(BSWAP32(this->unique_id), FGCM_EXACT, this->md5sum); - tmp = gc != nullptr ? gc->filename : nullptr; + tmp = gc != nullptr ? gc->filename.c_str() : nullptr; break; } case CONTENT_TYPE_BASE_GRAPHICS: diff --git a/src/newgrf.cpp b/src/newgrf.cpp index 3921208ef0..4d343468d9 100644 --- a/src/newgrf.cpp +++ b/src/newgrf.cpp @@ -414,10 +414,10 @@ static GRFFile *GetFileByGRFID(uint32 grfid) * @param filename The filename to obtain the file for. * @return The file. */ -static GRFFile *GetFileByFilename(const char *filename) +static GRFFile *GetFileByFilename(const std::string &filename) { for (GRFFile * const file : _grf_files) { - if (strcmp(file->filename, filename) == 0) return file; + if (file->filename == filename) return file; } return nullptr; } @@ -8882,7 +8882,7 @@ static void InitNewGRFFile(const GRFConfig *config) */ GRFFile::GRFFile(const GRFConfig *config) { - this->filename = stredup(config->filename); + this->filename = config->filename; this->grfid = config->ident.grfid; /* Initialise local settings to defaults */ @@ -8922,7 +8922,6 @@ GRFFile::GRFFile(const GRFConfig *config) GRFFile::~GRFFile() { - free(this->filename); delete[] this->language_map; } @@ -9186,7 +9185,7 @@ static void FinaliseCargoArray() * @param filename The filename of the newgrf this house was defined in. * @return Whether the given housespec is valid. */ -static bool IsHouseSpecValid(HouseSpec *hs, const HouseSpec *next1, const HouseSpec *next2, const HouseSpec *next3, const char *filename) +static bool IsHouseSpecValid(HouseSpec *hs, const HouseSpec *next1, const HouseSpec *next2, const HouseSpec *next3, const std::string &filename) { if (((hs->building_flags & BUILDING_HAS_2_TILES) != 0 && (next1 == nullptr || !next1->enabled || (next1->building_flags & BUILDING_HAS_1_TILE) != 0)) || @@ -9194,7 +9193,7 @@ static bool IsHouseSpecValid(HouseSpec *hs, const HouseSpec *next1, const HouseS (next2 == nullptr || !next2->enabled || (next2->building_flags & BUILDING_HAS_1_TILE) != 0 || next3 == nullptr || !next3->enabled || (next3->building_flags & BUILDING_HAS_1_TILE) != 0))) { hs->enabled = false; - if (filename != nullptr) Debug(grf, 1, "FinaliseHouseArray: {} defines house {} as multitile, but no suitable tiles follow. Disabling house.", filename, hs->grf_prop.local_id); + if (!filename.empty()) Debug(grf, 1, "FinaliseHouseArray: {} defines house {} as multitile, but no suitable tiles follow. Disabling house.", filename, hs->grf_prop.local_id); return false; } @@ -9204,13 +9203,13 @@ static bool IsHouseSpecValid(HouseSpec *hs, const HouseSpec *next1, const HouseS if (((hs->building_flags & BUILDING_HAS_2_TILES) != 0 && next1->population != 0) || ((hs->building_flags & BUILDING_HAS_4_TILES) != 0 && (next2->population != 0 || next3->population != 0))) { hs->enabled = false; - if (filename != nullptr) Debug(grf, 1, "FinaliseHouseArray: {} defines multitile house {} with non-zero population on additional tiles. Disabling house.", filename, hs->grf_prop.local_id); + if (!filename.empty()) Debug(grf, 1, "FinaliseHouseArray: {} defines multitile house {} with non-zero population on additional tiles. Disabling house.", filename, hs->grf_prop.local_id); return false; } /* Substitute type is also used for override, and having an override with a different size causes crashes. * This check should only be done for NewGRF houses because grf_prop.subst_id is not set for original houses.*/ - if (filename != nullptr && (hs->building_flags & BUILDING_HAS_1_TILE) != (HouseSpec::Get(hs->grf_prop.subst_id)->building_flags & BUILDING_HAS_1_TILE)) { + if (!filename.empty() && (hs->building_flags & BUILDING_HAS_1_TILE) != (HouseSpec::Get(hs->grf_prop.subst_id)->building_flags & BUILDING_HAS_1_TILE)) { hs->enabled = false; Debug(grf, 1, "FinaliseHouseArray: {} defines house {} with different house size then it's substitute type. Disabling house.", filename, hs->grf_prop.local_id); return false; @@ -9219,7 +9218,7 @@ static bool IsHouseSpecValid(HouseSpec *hs, const HouseSpec *next1, const HouseS /* Make sure that additional parts of multitile houses are not available. */ if ((hs->building_flags & BUILDING_HAS_1_TILE) == 0 && (hs->building_availability & HZ_ZONALL) != 0 && (hs->building_availability & HZ_CLIMALL) != 0) { hs->enabled = false; - if (filename != nullptr) Debug(grf, 1, "FinaliseHouseArray: {} defines house {} without a size but marked it as available. Disabling house.", filename, hs->grf_prop.local_id); + if (!filename.empty()) Debug(grf, 1, "FinaliseHouseArray: {} defines house {} without a size but marked it as available. Disabling house.", filename, hs->grf_prop.local_id); return false; } @@ -9297,7 +9296,7 @@ static void FinaliseHouseArray() /* We need to check all houses again to we are sure that multitile houses * did get consecutive IDs and none of the parts are missing. */ - if (!IsHouseSpecValid(hs, next1, next2, next3, nullptr)) { + if (!IsHouseSpecValid(hs, next1, next2, next3, std::string{})) { /* GetHouseNorthPart checks 3 houses that are directly before * it in the house pool. If any of those houses have multi-tile * flags set it assumes it's part of a multitile house. Since @@ -9606,7 +9605,7 @@ static void LoadNewGRFFileFromFile(GRFConfig *config, GrfLoadingStage stage, Spr */ void LoadNewGRFFile(GRFConfig *config, GrfLoadingStage stage, Subdirectory subdir, bool temporary) { - const char *filename = config->filename; + const std::string &filename = config->filename; /* A .grf file is activated only if it was active when the game was * started. If a game is loaded, only its active .grfs will be diff --git a/src/newgrf.h b/src/newgrf.h index 4110cb1243..af7af801d0 100644 --- a/src/newgrf.h +++ b/src/newgrf.h @@ -105,7 +105,7 @@ struct GRFLabel { /** Dynamic data of a loaded NewGRF */ struct GRFFile : ZeroedMemoryAllocator { - char *filename; + std::string filename; uint32 grfid; byte grf_version; diff --git a/src/newgrf_config.cpp b/src/newgrf_config.cpp index bb4ed080f2..77f82fcb2b 100644 --- a/src/newgrf_config.cpp +++ b/src/newgrf_config.cpp @@ -32,13 +32,11 @@ /** * Create a new GRFConfig. - * @param filename Set the filename of this GRFConfig to filename. The argument - * is copied so the original string isn't needed after the constructor. + * @param filename Set the filename of this GRFConfig to filename. */ -GRFConfig::GRFConfig(const char *filename) : - num_valid_params(lengthof(param)) +GRFConfig::GRFConfig(const std::string &filename) : + filename(filename), num_valid_params(lengthof(param)) { - if (filename != nullptr) this->filename = stredup(filename); } /** @@ -48,6 +46,7 @@ GRFConfig::GRFConfig(const char *filename) : GRFConfig::GRFConfig(const GRFConfig &config) : ZeroedMemoryAllocator(), ident(config.ident), + filename(config.filename), name(config.name), info(config.info), url(config.url), @@ -63,7 +62,6 @@ GRFConfig::GRFConfig(const GRFConfig &config) : { MemCpyT(this->original_md5sum, config.original_md5sum, lengthof(this->original_md5sum)); MemCpyT(this->param, config.param, lengthof(this->param)); - if (config.filename != nullptr) this->filename = stredup(config.filename); if (config.error != nullptr) this->error = std::make_unique(*config.error); for (uint i = 0; i < config.param_info.size(); i++) { if (config.param_info[i] == nullptr) { @@ -77,11 +75,6 @@ GRFConfig::GRFConfig(const GRFConfig &config) : /** Cleanup a GRFConfig object. */ GRFConfig::~GRFConfig() { - /* GCF_COPY as in NOT stredupped/alloced the filename */ - if (!HasBit(this->flags, GCF_COPY)) { - free(this->filename); - } - for (uint i = 0; i < this->param_info.size(); i++) delete this->param_info[i]; } @@ -104,7 +97,7 @@ void GRFConfig::CopyParams(const GRFConfig &src) const char *GRFConfig::GetName() const { const char *name = GetGRFStringFromGRFText(this->name); - return StrEmpty(name) ? this->filename : name; + return StrEmpty(name) ? this->filename.c_str() : name; } /** @@ -546,8 +539,7 @@ compatible_grf: * When the GCF_COPY flag is set, it is certain that the filename is * already a local one, so there is no need to replace it. */ if (!HasBit(c->flags, GCF_COPY)) { - free(c->filename); - c->filename = stredup(f->filename); + c->filename = f->filename; memcpy(c->ident.md5sum, f->ident.md5sum, sizeof(c->ident.md5sum)); c->name = f->name; c->info = f->name; @@ -644,7 +636,7 @@ bool GRFFileScanner::AddFile(const std::string &filename, size_t basepath_length const char *name = nullptr; if (c->name != nullptr) name = GetGRFStringFromGRFText(c->name); - if (name == nullptr) name = c->filename; + if (name == nullptr) name = c->filename.c_str(); UpdateNewGRFScanStatus(this->num_scanned, name); VideoDriver::GetInstance()->GameLoopPause(); @@ -797,5 +789,5 @@ char *GRFBuildParamList(char *dst, const GRFConfig *c, const char *last) */ const char *GRFConfig::GetTextfile(TextfileType type) const { - return ::GetTextfile(type, NEWGRF_DIR, this->filename); + return ::GetTextfile(type, NEWGRF_DIR, this->filename.c_str()); } diff --git a/src/newgrf_config.h b/src/newgrf_config.h index 5a089b7093..3e5a56739a 100644 --- a/src/newgrf_config.h +++ b/src/newgrf_config.h @@ -153,7 +153,7 @@ struct GRFParameterInfo { /** Information about GRF, used in the game and (part of it) in savegames */ struct GRFConfig : ZeroedMemoryAllocator { - GRFConfig(const char *filename = nullptr); + GRFConfig(const std::string &filename = std::string{}); GRFConfig(const GRFConfig &config); ~GRFConfig(); @@ -162,7 +162,7 @@ struct GRFConfig : ZeroedMemoryAllocator { GRFIdentifier ident; ///< grfid and md5sum to uniquely identify newgrfs uint8 original_md5sum[16]; ///< MD5 checksum of original file if only a 'compatible' file was loaded - char *filename; ///< Filename - either with or without full path + std::string filename; ///< Filename - either with or without full path GRFTextWrapper name; ///< NOSAVE: GRF name (Action 0x08) GRFTextWrapper info; ///< NOSAVE: GRF info (author, copyright, ...) (Action 0x08) GRFTextWrapper url; ///< NOSAVE: URL belonging to this GRF. diff --git a/src/newgrf_gui.cpp b/src/newgrf_gui.cpp index 0cd608de4a..63a326a975 100644 --- a/src/newgrf_gui.cpp +++ b/src/newgrf_gui.cpp @@ -86,7 +86,7 @@ static void ShowNewGRFInfo(const GRFConfig *c, const Rect &r, bool show_params) } /* Draw filename or not if it is not known (GRF sent over internet) */ - if (c->filename != nullptr) { + if (!c->filename.empty()) { SetDParamStr(0, c->filename); tr.top = DrawStringMultiLine(tr, STR_NEWGRF_SETTINGS_FILENAME); } @@ -1442,7 +1442,7 @@ private: { filter.ResetState(); filter.AddLine((*a)->GetName()); - filter.AddLine((*a)->filename); + filter.AddLine((*a)->filename.c_str()); filter.AddLine((*a)->GetDescription()); return filter.GetState();; }