Skip to content

Commit

Permalink
IniFile: Store sections in unique_ptrs, instead of directly.
Browse files Browse the repository at this point in the history
This fixes an issue when you create two sections consecutively and
retain pointers to them, and then modify them, such as happens in the
postshader ini initialization. Previously, one of the section pointers
could get invalidated since the section vector got resized. Now, the
pointed-to sections don't move around in memory, only the list of them
does.
  • Loading branch information
hrydgard committed Aug 13, 2023
1 parent 2a74a0b commit d82ecf1
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 39 deletions.
57 changes: 25 additions & 32 deletions Common/Data/Format/IniFile.cpp
Expand Up @@ -420,51 +420,44 @@ bool Section::Delete(const char *key)

// IniFile

const Section* IniFile::GetSection(const char* sectionName) const
{
for (std::vector<Section>::const_iterator iter = sections.begin(); iter != sections.end(); ++iter)
const Section* IniFile::GetSection(const char* sectionName) const {
for (const auto &iter : sections)
if (!strcasecmp(iter->name().c_str(), sectionName))
return (&(*iter));
return 0;
return iter.get();
return nullptr ;
}

Section* IniFile::GetSection(const char* sectionName)
{
for (std::vector<Section>::iterator iter = sections.begin(); iter != sections.end(); ++iter)
Section* IniFile::GetSection(const char* sectionName) {
for (const auto &iter : sections)
if (!strcasecmp(iter->name().c_str(), sectionName))
return (&(*iter));
return iter.get();
return 0;
}

Section* IniFile::GetOrCreateSection(const char* sectionName)
{
Section* IniFile::GetOrCreateSection(const char* sectionName) {
Section* section = GetSection(sectionName);
if (!section)
{
sections.push_back(Section(sectionName));
section = &sections[sections.size() - 1];
if (!section) {
sections.push_back(std::unique_ptr<Section>(new Section(sectionName)));
section = sections.back().get();
}
return section;
}

bool IniFile::DeleteSection(const char* sectionName)
{
bool IniFile::DeleteSection(const char* sectionName) {
Section* s = GetSection(sectionName);
if (!s)
return false;
for (std::vector<Section>::iterator iter = sections.begin(); iter != sections.end(); ++iter)
{
if (&(*iter) == s)
{

for (auto iter = sections.begin(); iter != sections.end(); ++iter) {
if (iter->get() == s) {
sections.erase(iter);
return true;
}
}
return false;
}

bool IniFile::Exists(const char* sectionName, const char* key) const
{
bool IniFile::Exists(const char* sectionName, const char* key) const {
const Section* section = GetSection(sectionName);
if (!section)
return false;
Expand Down Expand Up @@ -556,7 +549,7 @@ void IniFile::SortSections()
bool IniFile::Load(const Path &path)
{
sections.clear();
sections.push_back(Section(""));
sections.push_back(std::unique_ptr<Section>(new Section("")));
// first section consists of the comments before the first real section

// Open file
Expand Down Expand Up @@ -612,16 +605,16 @@ bool IniFile::Load(std::istream &in) {
if (sectionNameEnd != std::string::npos) {
// New section!
std::string sub = line.substr(1, sectionNameEnd - 1);
sections.push_back(Section(sub));
sections.push_back(std::unique_ptr<Section>(new Section(sub)));

if (sectionNameEnd + 1 < line.size()) {
sections[sections.size() - 1].comment = line.substr(sectionNameEnd + 1);
sections.back()->comment = line.substr(sectionNameEnd + 1);
}
} else {
if (sections.empty()) {
sections.push_back(Section(""));
sections.push_back(std::unique_ptr<Section>(new Section("")));
}
sections[sections.size() - 1].lines.push_back(line);
sections.back()->lines.push_back(line);
}
}
}
Expand All @@ -641,12 +634,12 @@ bool IniFile::Save(const Path &filename)
// TODO: Do we still need this? It's annoying.
fprintf(file, "\xEF\xBB\xBF");

for (const Section &section : sections) {
if (!section.name().empty() && (!section.lines.empty() || !section.comment.empty())) {
fprintf(file, "[%s]%s\n", section.name().c_str(), section.comment.c_str());
for (const auto &section : sections) {
if (!section->name().empty() && (!section->lines.empty() || !section->comment.empty())) {
fprintf(file, "[%s]%s\n", section->name().c_str(), section->comment.c_str());
}

for (const std::string &s : section.lines) {
for (const std::string &s : section->lines) {
fprintf(file, "%s\n", s.c_str());
}
}
Expand Down
6 changes: 4 additions & 2 deletions Common/Data/Format/IniFile.h
Expand Up @@ -5,6 +5,7 @@
#pragma once

#include <istream>
#include <memory>
#include <map>
#include <string>
#include <vector>
Expand Down Expand Up @@ -144,14 +145,15 @@ class IniFile {
bool DeleteSection(const char* sectionName);

void SortSections();
std::vector<Section> &Sections() { return sections; }

std::vector<std::unique_ptr<Section>> &Sections() { return sections; }

bool HasSection(const char *section) { return GetSection(section) != 0; }

Section* GetOrCreateSection(const char* section);

private:
std::vector<Section> sections;
std::vector<std::unique_ptr<Section>> sections;

const Section* GetSection(const char* section) const;
Section* GetSection(const char* section);
Expand Down
6 changes: 3 additions & 3 deletions Common/Data/Text/I18n.cpp
Expand Up @@ -140,13 +140,13 @@ bool I18NRepo::LoadIni(const std::string &languageID, const Path &overridePath)

Clear();

const std::vector<Section> &sections = ini.Sections();
const std::vector<std::unique_ptr<Section>> &sections = ini.Sections();

std::lock_guard<std::mutex> guard(catsLock_);
for (auto &section : sections) {
for (size_t i = 0; i < (size_t)I18NCat::CATEGORY_COUNT; i++) {
if (!strcmp(section.name().c_str(), g_categoryNames[i])) {
cats_[i].reset(new I18NCategory(section));
if (!strcmp(section->name().c_str(), g_categoryNames[i])) {
cats_[i].reset(new I18NCategory(*section.get()));
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion GPU/Common/PostShader.cpp
Expand Up @@ -100,7 +100,7 @@ void LoadPostShaderInfo(Draw::DrawContext *draw, const std::vector<Path> &direct

// Alright, let's loop through the sections and see if any is a shader.
for (size_t i = 0; i < ini.Sections().size(); i++) {
Section &section = ini.Sections()[i];
Section &section = *(ini.Sections()[i].get());
std::string shaderType;
section.Get("Type", &shaderType, "render");

Expand Down
2 changes: 1 addition & 1 deletion UI/Theme.cpp
Expand Up @@ -113,7 +113,7 @@ static void LoadThemeInfo(const std::vector<Path> &directories) {

// Alright, let's loop through the sections and see if any is a theme.
for (size_t i = 0; i < ini.Sections().size(); i++) {
Section &section = ini.Sections()[i];
Section &section = *(ini.Sections()[i].get());
ThemeInfo info;
section.Get("Name", &info.name, section.name().c_str());

Expand Down

0 comments on commit d82ecf1

Please sign in to comment.