cpp/openttd-patchpack/source Changeset - r28015:d55e2fd5a748

Changeset - r28015:d55e2fd5a748

Parent rev.

Child rev.

[Not reviewed]

master

0 6 0

Peter Nelson - 7 months ago 2023-10-10 18:26:00
peter1138@openttd.org

Codechange: Use std::list instead of C-linked list for ini files.

This removes self-management of memory with new/delete and allows simpler iteration.

6 files changed with 70 insertions and 143 deletions:

src/base_media_func.h

src/ini.cpp

src/ini_load.cpp

src/ini_type.h

src/settings.cpp

src/settingsgen/settingsgen.cpp

0 comments (0 inline, 0 general)

src/base_media_func.h

➞

Show inline comments

@@ @@ -56,10 +56,10 @@ bool BaseSet<T, Tnum_files, Tsearch_in_t @@
 	this->description[std::string{}] = *item->value;
 	/* Add the translations of the descriptions too. */
 	for (item = metadata->item; item != nullptr; item = item->next) {
 		if (item->name.compare(0, 12, "description.") != 0) continue;
 	for (const IniItem &titem : metadata->items) {
 		if (titem.name.compare(0, 12, "description.") != 0) continue;
-		this->description[item->name.substr(12)] = item->value.value_or("");
+		this->description[titem.name.substr(12)] = titem.value.value_or("");
+	}
 	fetch_metadata("shortname");

src/ini.cpp

➞

Show inline comments

@@ @@ -56,20 +56,20 @@ bool IniFile::SaveToDisk(const std::stri @@
 	std::ofstream os(OTTD2FS(file_new).c_str());
 	if (os.fail()) return false;
 	for (const IniGroup *group = this->group; group != nullptr; group = group->next) {
 		os << group->comment << "[" << group->name << "]\n";
 		for (const IniItem *item = group->item; item != nullptr; item = item->next) {
 			os << item->comment;
 	for (const IniGroup &group : this->groups) {
 		os << group.comment << "[" << group.name << "]\n";
 		for (const IniItem &item : group.items) {
 			os << item.comment;
 			/* protect item->name with quotes if needed */
 			if (item->name.find(' ') != std::string::npos ||
 				item->name[0] == '[') {
 				os << "\"" << item->name << "\"";
 			if (item.name.find(' ') != std::string::npos ||
 				item.name[0] == '[') {
 				os << "\"" << item.name << "\"";
 			} else {
-				os << item->name;
+				os << item.name;
+			}
-			os << " = " << item->value.value_or("") << "\n";
+			os << " = " << item.value.value_or("") << "\n";
+		}
+	}
 	os << this->comment;

src/ini_load.cpp

➞

Show inline comments

@@ @@ -20,18 +20,9 @@ @@
  * @param parent the group we belong to
  * @param name   the name of the item
  */
-IniItem::IniItem(IniGroup *parent, const std::string &name) : next(nullptr)
 IniItem::IniItem(const std::string &name)
+{
 	this->name = StrMakeValid(name);
 	*parent->last_item = this;
 	parent->last_item = &this->next;
+}
 /** Free everything we loaded. */
 IniItem::~IniItem()
+{
 	delete this->next;
+}
 /**
@@ @@ -48,20 +39,9 @@ void IniItem::SetValue(const std::string @@
  * @param parent the file we belong to
  * @param name   the name of the group
  */
-IniGroup::IniGroup(IniLoadFile *parent, const std::string &name, IniGroupType type) : next(nullptr), type(type), item(nullptr)
 IniGroup::IniGroup(const std::string &name, IniGroupType type) : type(type)
+{
 	this->name = StrMakeValid(name);
 	this->last_item = &this->item;
 	*parent->last_group = this;
 	parent->last_group = &this->next;
+}
 /** Free everything we loaded. */
 IniGroup::~IniGroup()
+{
 	delete this->item;
 	delete this->next;
+}
 /**
@@ @@ -69,10 +49,10 @@ IniGroup::~IniGroup() @@
  * @param name   name of the item to find.
  * @return the requested item or nullptr if not found.
  */
-IniItem *IniGroup::GetItem(const std::string &name) const
 IniItem *IniGroup::GetItem(const std::string &name)
+{
 	for (IniItem *item = this->item; item != nullptr; item = item->next) {
 		if (item->name == name) return item;
 	for (IniItem &item : this->items) {
 		if (item.name == name) return &item;
+	}
 	return nullptr;
@@ @@ -85,8 +65,8 @@ IniItem *IniGroup::GetItem(const std::st @@
  */
 IniItem &IniGroup::GetOrCreateItem(const std::string &name)
+{
 	for (IniItem *item = this->item; item != nullptr; item = item->next) {
 		if (item->name == name) return *item;
 	for (IniItem &item : this->items) {
 		if (item.name == name) return item;
+	}
 	/* Item doesn't exist, make a new one. */
@@ @@ -100,7 +80,7 @@ IniItem &IniGroup::GetOrCreateItem(const @@
  */
 IniItem &IniGroup::CreateItem(const std::string &name)
+{
-	return *(new IniItem(this, name));
+	return this->items.emplace_back(name);
+}
 /**
@@ @@ -109,22 +89,7 @@ IniItem &IniGroup::CreateItem(const std: @@
  */
 void IniGroup::RemoveItem(const std::string &name)
+{
 	IniItem **prev = &this->item;
 	for (IniItem *item = this->item; item != nullptr; prev = &item->next, item = item->next) {
 		if (item->name != name) continue;
 		*prev = item->next;
 		/* "last_item" is a pointer to the "real-last-item"->next. */
 		if (this->last_item == &item->next) {
 			this->last_item = prev;
+		}
 		item->next = nullptr;
 		delete item;
 		return;
+	}
 	this->items.remove_if([&name](const IniItem &item) { return item.name == name; });
+}
 /**
@@ @@ -132,9 +97,7 @@ void IniGroup::RemoveItem(const std::str @@
  */
 void IniGroup::Clear()
+{
 	delete this->item;
 	this->item = nullptr;
 	this->last_item = &this->item;
 	this->items.clear();
+}
 /**
@@ @@ -143,17 +106,9 @@ void IniGroup::Clear() @@
  * @param seq_group_names  A list with group names that should be loaded as lists of names. @see IGT_SEQUENCE
  */
 IniLoadFile::IniLoadFile(const IniGroupNameList &list_group_names, const IniGroupNameList &seq_group_names) :
 		group(nullptr),
 		list_group_names(list_group_names),
 		seq_group_names(seq_group_names)
+{
 	this->last_group = &this->group;
+}
 /** Free everything we loaded. */
 IniLoadFile::~IniLoadFile()
+{
 	delete this->group;
+}
 /**
@@ @@ -161,10 +116,10 @@ IniLoadFile::~IniLoadFile() @@
  * @param name name of the group to find.
  * @return The requested group or \c nullptr if not found.
  */
-IniGroup *IniLoadFile::GetGroup(const std::string &name) const
 IniGroup *IniLoadFile::GetGroup(const std::string &name)
+{
 	for (IniGroup *group = this->group; group != nullptr; group = group->next) {
 		if (group->name == name) return group;
 	for (IniGroup &group : this->groups) {
 		if (group.name == name) return &group;
+	}
 	return nullptr;
@@ @@ -177,8 +132,8 @@ IniGroup *IniLoadFile::GetGroup(const st @@
  */
 IniGroup &IniLoadFile::GetOrCreateGroup(const std::string &name)
+{
 	for (IniGroup *group = this->group; group != nullptr; group = group->next) {
 		if (group->name == name) return *group;
 	for (IniGroup &group : this->groups) {
 		if (group.name == name) return group;
+	}
 	/* Group doesn't exist, make a new one. */
@@ @@ -196,9 +151,7 @@ IniGroup &IniLoadFile::CreateGroup(const @@
 	if (std::find(this->list_group_names.begin(), this->list_group_names.end(), name) != this->list_group_names.end()) type = IGT_LIST;
 	if (std::find(this->seq_group_names.begin(), this->seq_group_names.end(), name) != this->seq_group_names.end()) type = IGT_SEQUENCE;
 	IniGroup *group = new IniGroup(this, name, type);
 	group->comment = "\n";
 	return *group;
 	return this->groups.emplace_back(name, type);
+}
 /**
@@ @@ -208,28 +161,7 @@ IniGroup &IniLoadFile::CreateGroup(const @@
 void IniLoadFile::RemoveGroup(const std::string &name)
+{
 	size_t len = name.length();
 	IniGroup *prev = nullptr;
 	IniGroup *group;
 	/* does it exist already? */
 	for (group = this->group; group != nullptr; prev = group, group = group->next) {
 		if (group->name.compare(0, len, name) == 0) {
 			break;
+		}
+	}
 	if (group == nullptr) return;
 	if (prev != nullptr) {
 		prev->next = prev->next->next;
 		if (this->last_group == &group->next) this->last_group = &prev->next;
 	} else {
 		this->group = this->group->next;
 		if (this->last_group == &group->next) this->last_group = &this->group;
+	}
 	group->next = nullptr;
 	delete group;
 	this->groups.remove_if([&name, &len](const IniGroup &group) { return group.name.compare(0, len, name) == 0; });
+}
 /**
@@ @@ -240,7 +172,7 @@ void IniLoadFile::RemoveGroup(const std: @@
  */
 void IniLoadFile::LoadFromDisk(const std::string &filename, Subdirectory subdir)
+{
-	assert(this->last_group == &this->group);
+	assert(this->groups.empty());
 	char buffer[1024];
 	IniGroup *group = nullptr;

src/ini_type.h

➞

Show inline comments

@@ @@ -21,30 +21,25 @@ enum IniGroupType { @@
 /** A single "line" in an ini file. */
 struct IniItem {
 	IniItem *next;                    ///< The next item in this group
 	std::string name;                 ///< The name of this item
 	std::optional<std::string> value; ///< The value of this item
 	std::string comment;              ///< The comment associated with this item
 	IniItem(struct IniGroup *parent, const std::string &name);
 	~IniItem();
 	IniItem(const std::string &name);
 	void SetValue(const std::string_view value);
 };
 /** A group within an ini file. */
 struct IniGroup {
-	IniGroup *next;      ///< the next group within this file
+	std::list<IniItem> items; ///< all items in the group
 	IniGroupType type;   ///< type of group
 	IniItem *item;       ///< the first item in the group
 	IniItem **last_item; ///< the last item in the group
 	std::string name;    ///< name of group
 	std::string comment; ///< comment for group
 	IniGroup(struct IniLoadFile *parent, const std::string &name, IniGroupType type);
 	~IniGroup();
 	IniGroup(const std::string &name, IniGroupType type);
-	IniItem *GetItem(const std::string &name) const;
 	IniItem *GetItem(const std::string &name);
 	IniItem &GetOrCreateItem(const std::string &name);
 	IniItem &CreateItem(const std::string &name);
 	void RemoveItem(const std::string &name);
@@ @@ -55,16 +50,15 @@ struct IniGroup { @@
 struct IniLoadFile {
 	using IniGroupNameList = std::initializer_list<std::string_view>;
 	IniGroup *group;                      ///< the first group in the ini
 	IniGroup **last_group;                ///< the last group in the ini
 	std::list<IniGroup> groups; ///< all groups in the ini
 	std::string comment;                  ///< last comment in file
 	const IniGroupNameList list_group_names; ///< list of group names that are lists
 	const IniGroupNameList seq_group_names;  ///< list of group names that are sequences.
 	IniLoadFile(const IniGroupNameList &list_group_names = {}, const IniGroupNameList &seq_group_names = {});
-	virtual ~IniLoadFile();
+	virtual ~IniLoadFile() { }
-	IniGroup *GetGroup(const std::string &name) const;
 	IniGroup *GetGroup(const std::string &name);
 	IniGroup &GetOrCreateGroup(const std::string &name);
 	IniGroup &CreateGroup(const std::string &name);
 	void RemoveGroup(const std::string &name);

src/settings.cpp

➞

Show inline comments

@@ @@ -786,8 +786,8 @@ static void IniLoadSettingList(IniFile & @@
 	list.clear();
 	for (const IniItem *item = group->item; item != nullptr; item = item->next) {
 		if (!item->name.empty()) list.push_back(item->name);
 	for (const IniItem &item : group->items) {
 		if (!item.name.empty()) list.push_back(item.name);
+	}
+}
@@ @@ -907,17 +907,19 @@ static void AILoadConfig(IniFile &ini, c @@
 	if (group == nullptr) return;
 	CompanyID c = COMPANY_FIRST;
-	for (IniItem *item = group->item; c < MAX_COMPANIES && item != nullptr; c++, item = item->next) {
+	for (const IniItem &item : group->items) {
 		AIConfig *config = AIConfig::GetConfig(c, AIConfig::SSS_FORCE_NEWGAME);
-		config->Change(item->name);
+		config->Change(item.name);
 		if (!config->HasScript()) {
 			if (item->name != "none") {
 				Debug(script, 0, "The AI by the name '{}' was no longer found, and removed from the list.", item->name);
 			if (item.name != "none") {
 				Debug(script, 0, "The AI by the name '{}' was no longer found, and removed from the list.", item.name);
 				continue;
+			}
+		}
 		if (item->value.has_value()) config->StringToSettings(*item->value);
 		if (item.value.has_value()) config->StringToSettings(*item.value);
 		c++;
 		if (c >= MAX_COMPANIES) break;
+	}
+}
@@ @@ -929,21 +931,20 @@ static void GameLoadConfig(IniFile &ini, @@
 	GameConfig::GetConfig(GameConfig::SSS_FORCE_NEWGAME)->Change(std::nullopt);
 	/* If no group exists, return */
 	if (group == nullptr) return;
+	if (group == nullptr || group->items.empty()) return;
 	IniItem *item = group->item;
 	if (item == nullptr) return;
 	IniItem &item = group->items.front();
 	GameConfig *config = GameConfig::GetConfig(AIConfig::SSS_FORCE_NEWGAME);
-	config->Change(item->name);
+	config->Change(item.name);
 	if (!config->HasScript()) {
 		if (item->name != "none") {
 			Debug(script, 0, "The GameScript by the name '{}' was no longer found, and removed from the list.", item->name);
 		if (item.name != "none") {
 			Debug(script, 0, "The GameScript by the name '{}' was no longer found, and removed from the list.", item.name);
 			return;
+		}
+	}
-	if (item->value.has_value()) config->StringToSettings(*item->value);
+	if (item.value.has_value()) config->StringToSettings(*item.value);
+}
 /**
@@ @@ -995,12 +996,12 @@ static GRFConfig *GRFLoadConfig(IniFile @@
 	if (group == nullptr) return nullptr;
 	uint num_grfs = 0;
-	for (IniItem *item = group->item; item != nullptr; item = item->next) {
+	for (const IniItem &item : group->items) {
 		GRFConfig *c = nullptr;
 		uint8_t grfid_buf[4];
 		MD5Hash md5sum;
-		const char *filename = item->name.c_str();
+		const char *filename = item.name.c_str();
 		bool has_grfid = false;
 		bool has_md5sum = false;
@@ @@ -1024,8 +1025,8 @@ static GRFConfig *GRFLoadConfig(IniFile @@
 		if (c == nullptr) c = new GRFConfig(filename);
 		/* Parse parameters */
 		if (item->value.has_value() && !item->value->empty()) {
 			int count = ParseIntList(item->value->c_str(), c->param.data(), c->param.size());
 		if (item.value.has_value() && !item.value->empty()) {
 			int count = ParseIntList(item.value->c_str(), c->param.data(), c->param.size());
 			if (count < 0) {
 				SetDParamStr(0, filename);
 				ShowErrorMessage(STR_CONFIG_ERROR, STR_CONFIG_ERROR_ARRAY, WL_CRITICAL);
@@ @@ -1048,7 +1049,7 @@ static GRFConfig *GRFLoadConfig(IniFile @@
 				SetDParam(1, STR_CONFIG_ERROR_INVALID_GRF_UNKNOWN);
+			}
-			SetDParamStr(0, StrEmpty(filename) ? item->name.c_str() : filename);
+			SetDParamStr(0, StrEmpty(filename) ? item.name.c_str() : filename);
 			ShowErrorMessage(STR_CONFIG_ERROR, STR_CONFIG_ERROR_INVALID_GRF, WL_CRITICAL);
 			delete c;
 			continue;
@@ @@ -1438,9 +1439,9 @@ StringList GetGRFPresetList() @@
 	StringList list;
 	ConfigIniFile ini(_config_file);
 	for (IniGroup *group = ini.group; group != nullptr; group = group->next) {
 		if (group->name.compare(0, 7, "preset-") == 0) {
 			list.push_back(group->name.substr(7));
 	for (const IniGroup &group : ini.groups) {
 		if (group.name.compare(0, 7, "preset-") == 0) {
 			list.push_back(group.name.substr(7));
+		}
+	}

src/settingsgen/settingsgen.cpp

➞

Show inline comments

@@ @@ -197,9 +197,9 @@ static void DumpGroup(IniLoadFile &ifile @@
+{
 	IniGroup *grp = ifile.GetGroup(group_name);
 	if (grp != nullptr && grp->type == IGT_SEQUENCE) {
 		for (IniItem *item = grp->item; item != nullptr; item = item->next) {
 			if (!item->name.empty()) {
 				_stored_output.Add(item->name.c_str());
 		for (IniItem &item : grp->items) {
 			if (!item.name.empty()) {
 				_stored_output.Add(item.name.c_str());
 				_stored_output.Add("\n", 1);
+			}
+		}
@@ @@ -302,21 +302,21 @@ static void DumpSections(IniLoadFile &if @@
 	if (templates_grp == nullptr) return;
 	/* Output every group, using its name as template name. */
-	for (IniGroup *grp = ifile.group; grp != nullptr; grp = grp->next) {
+	for (IniGroup &grp : ifile.groups) {
 		const char * const *sgn;
-		for (sgn = special_group_names; *sgn != nullptr; sgn++) if (grp->name == *sgn) break;
+		for (sgn = special_group_names; *sgn != nullptr; sgn++) if (grp.name == *sgn) break;
 		if (*sgn != nullptr) continue;
-		IniItem *template_item = templates_grp->GetItem(grp->name); // Find template value.
+		IniItem *template_item = templates_grp->GetItem(grp.name); // Find template value.
 		if (template_item == nullptr || !template_item->value.has_value()) {
-			FatalError("Cannot find template {}", grp->name);
+			FatalError("Cannot find template {}", grp.name);
+		}
 		DumpLine(template_item, grp, default_grp, _stored_output);
+		DumpLine(template_item, &grp, default_grp, _stored_output);
 		if (validation_grp != nullptr) {
-			IniItem *validation_item = validation_grp->GetItem(grp->name); // Find template value.
+			IniItem *validation_item = validation_grp->GetItem(grp.name); // Find template value.
 			if (validation_item != nullptr && validation_item->value.has_value()) {
 				DumpLine(validation_item, grp, default_grp, _post_amble_output);
+				DumpLine(validation_item, &grp, default_grp, _post_amble_output);
+			}
+		}
+	}

0 comments (0 inline, 0 general)