From de7e9557dceb29de00b301a56df81a6ca419c772 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 16 Jun 2019 16:57:05 -0400 Subject: [PATCH 1/2] Common/IniFile: Make CaseInsensitiveStringCompare usable with heterogenous lookup Previously, when performing find() operations or indexing operations on the section map, it would need to operate on a std::string key. This means cases like: map.find(some_string_view) aren't usable, which kind of sucks, especially given for most cases, we use regular string literals to perform operations in calling code. However, since C++14, it's possible to use heterogenous lookup to avoid needing to construct exact key types. In otherwords, we can perform the above or use string literals without constructing a std::string instance around them implicitly. We simply need to specify a member type within our comparison struct named is_transparent, to allow std::map to perform automatic type deduction. We also slightly alter the algorithm to an equivalent compatible with std::string_view (which need not be null-terminated), as strcasecmp requires null-terminated strings. While we're at it, we can also provide a helper function to the struct for comparing string equality rather than only less than. This allows removing other usages of strcasecmp in other functions, allowing for the transition of them to std::string_view. --- Source/Core/Common/IniFile.cpp | 17 ++++++++++------- Source/Core/Common/IniFile.h | 25 +++++++++++++++++++++---- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/Source/Core/Common/IniFile.cpp b/Source/Core/Common/IniFile.cpp index 65deaa1573..74f43dc165 100644 --- a/Source/Core/Common/IniFile.cpp +++ b/Source/Core/Common/IniFile.cpp @@ -5,16 +5,13 @@ #include "Common/IniFile.h" #include -#include #include -#include #include #include #include #include #include -#include "Common/CommonTypes.h" #include "Common/FileUtil.h" #include "Common/StringUtil.h" @@ -128,16 +125,22 @@ IniFile::~IniFile() = default; const IniFile::Section* IniFile::GetSection(const std::string& sectionName) const { for (const Section& sect : sections) - if (!strcasecmp(sect.name.c_str(), sectionName.c_str())) - return (&(sect)); + { + if (CaseInsensitiveStringCompare::IsEqual(sect.name, sectionName)) + return § + } + return nullptr; } IniFile::Section* IniFile::GetSection(const std::string& sectionName) { for (Section& sect : sections) - if (!strcasecmp(sect.name.c_str(), sectionName.c_str())) - return (&(sect)); + { + if (CaseInsensitiveStringCompare::IsEqual(sect.name, sectionName)) + return § + } + return nullptr; } diff --git a/Source/Core/Common/IniFile.h b/Source/Core/Common/IniFile.h index f23a52fcb0..3b87501d29 100644 --- a/Source/Core/Common/IniFile.h +++ b/Source/Core/Common/IniFile.h @@ -4,21 +4,38 @@ #pragma once -#include +#include +#include #include #include #include +#include #include -#include "Common/CommonFuncs.h" #include "Common/CommonTypes.h" #include "Common/StringUtil.h" struct CaseInsensitiveStringCompare { - bool operator()(const std::string& a, const std::string& b) const + // Allow heterogenous lookup. + using is_transparent = void; + + bool operator()(std::string_view a, std::string_view b) const { - return strcasecmp(a.c_str(), b.c_str()) < 0; + return std::lexicographical_compare( + a.begin(), a.end(), b.begin(), b.end(), [](char lhs, char rhs) { + return std::tolower(static_cast(lhs)) < std::tolower(static_cast(rhs)); + }); + } + + static bool IsEqual(std::string_view a, std::string_view b) + { + if (a.size() != b.size()) + return false; + + return std::equal(a.begin(), a.end(), b.begin(), b.end(), [](char lhs, char rhs) { + return std::tolower(static_cast(lhs)) == std::tolower(static_cast(rhs)); + }); } }; From 78d171625186eb1018f8827d48e3cc3a9185bca2 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 16 Jun 2019 17:28:00 -0400 Subject: [PATCH 2/2] Common/IniFile: Make use of std::string_view where applicable Now that the std::map less-than comparitor is capable of being used with heterogenous lookup, we're able to convert many of the querying functions that took std::string references over to std::string_view. Now these functions may be used without potentially allocating a std::string instance unnecessarily. --- Source/Core/Common/IniFile.cpp | 61 ++++++++++++++++++---------------- Source/Core/Common/IniFile.h | 46 ++++++++++++------------- 2 files changed, 55 insertions(+), 52 deletions(-) diff --git a/Source/Core/Common/IniFile.cpp b/Source/Core/Common/IniFile.cpp index 74f43dc165..c0b4f2be28 100644 --- a/Source/Core/Common/IniFile.cpp +++ b/Source/Core/Common/IniFile.cpp @@ -51,32 +51,34 @@ void IniFile::Section::Set(const std::string& key, std::string new_value) keys_order.push_back(key); } -bool IniFile::Section::Get(const std::string& key, std::string* value, - const std::string& defaultValue) const +bool IniFile::Section::Get(std::string_view key, std::string* value, + const std::string& default_value) const { - auto it = values.find(key); + const auto it = values.find(key); + if (it != values.end()) { *value = it->second; return true; } - else if (&defaultValue != &NULL_STRING) + + if (&default_value != &NULL_STRING) { - *value = defaultValue; + *value = default_value; return true; } return false; } -bool IniFile::Section::Exists(const std::string& key) const +bool IniFile::Section::Exists(std::string_view key) const { return values.find(key) != values.end(); } -bool IniFile::Section::Delete(const std::string& key) +bool IniFile::Section::Delete(std::string_view key) { - auto it = values.find(key); + const auto it = values.find(key); if (it == values.end()) return false; @@ -122,44 +124,45 @@ IniFile::IniFile() = default; IniFile::~IniFile() = default; -const IniFile::Section* IniFile::GetSection(const std::string& sectionName) const +const IniFile::Section* IniFile::GetSection(std::string_view section_name) const { for (const Section& sect : sections) { - if (CaseInsensitiveStringCompare::IsEqual(sect.name, sectionName)) + if (CaseInsensitiveStringCompare::IsEqual(sect.name, section_name)) return § } return nullptr; } -IniFile::Section* IniFile::GetSection(const std::string& sectionName) +IniFile::Section* IniFile::GetSection(std::string_view section_name) { for (Section& sect : sections) { - if (CaseInsensitiveStringCompare::IsEqual(sect.name, sectionName)) + if (CaseInsensitiveStringCompare::IsEqual(sect.name, section_name)) return § } return nullptr; } -IniFile::Section* IniFile::GetOrCreateSection(const std::string& sectionName) +IniFile::Section* IniFile::GetOrCreateSection(std::string_view section_name) { - Section* section = GetSection(sectionName); + Section* section = GetSection(section_name); if (!section) { - sections.emplace_back(sectionName); + sections.emplace_back(std::string(section_name)); section = §ions.back(); } return section; } -bool IniFile::DeleteSection(const std::string& sectionName) +bool IniFile::DeleteSection(std::string_view section_name) { - Section* s = GetSection(sectionName); + Section* s = GetSection(section_name); if (!s) return false; + for (auto iter = sections.begin(); iter != sections.end(); ++iter) { if (&(*iter) == s) @@ -168,41 +171,43 @@ bool IniFile::DeleteSection(const std::string& sectionName) return true; } } + return false; } -bool IniFile::Exists(const std::string& sectionName, const std::string& key) const +bool IniFile::Exists(std::string_view section_name, std::string_view key) const { - const Section* section = GetSection(sectionName); + const Section* section = GetSection(section_name); if (!section) return false; + return section->Exists(key); } -void IniFile::SetLines(const std::string& sectionName, const std::vector& lines) +void IniFile::SetLines(std::string_view section_name, const std::vector& lines) { - Section* section = GetOrCreateSection(sectionName); + Section* section = GetOrCreateSection(section_name); section->SetLines(lines); } -void IniFile::SetLines(const std::string& section_name, std::vector&& lines) +void IniFile::SetLines(std::string_view section_name, std::vector&& lines) { Section* section = GetOrCreateSection(section_name); section->SetLines(std::move(lines)); } -bool IniFile::DeleteKey(const std::string& sectionName, const std::string& key) +bool IniFile::DeleteKey(std::string_view section_name, std::string_view key) { - Section* section = GetSection(sectionName); + Section* section = GetSection(section_name); if (!section) return false; return section->Delete(key); } // Return a list of all keys in a section -bool IniFile::GetKeys(const std::string& sectionName, std::vector* keys) const +bool IniFile::GetKeys(std::string_view section_name, std::vector* keys) const { - const Section* section = GetSection(sectionName); + const Section* section = GetSection(section_name); if (!section) { return false; @@ -212,12 +217,12 @@ bool IniFile::GetKeys(const std::string& sectionName, std::vector* } // Return a list of all lines in a section -bool IniFile::GetLines(const std::string& sectionName, std::vector* lines, +bool IniFile::GetLines(std::string_view section_name, std::vector* lines, const bool remove_comments) const { lines->clear(); - const Section* section = GetSection(sectionName); + const Section* section = GetSection(section_name); if (!section) return false; diff --git a/Source/Core/Common/IniFile.h b/Source/Core/Common/IniFile.h index 3b87501d29..38afa2631c 100644 --- a/Source/Core/Common/IniFile.h +++ b/Source/Core/Common/IniFile.h @@ -49,8 +49,8 @@ public: public: Section(); explicit Section(std::string name_); - bool Exists(const std::string& key) const; - bool Delete(const std::string& key); + bool Exists(std::string_view key) const; + bool Delete(std::string_view key); void Set(const std::string& key, std::string new_value); @@ -69,12 +69,11 @@ public: Delete(key); } - bool Get(const std::string& key, std::string* value, + bool Get(std::string_view key, std::string* value, const std::string& default_value = NULL_STRING) const; template - bool Get(const std::string& key, T* value, - const std::common_type_t& default_value = {}) const + bool Get(std::string_view key, T* value, const std::common_type_t& default_value = {}) const { std::string temp; bool retval = Get(key, &temp); @@ -121,41 +120,40 @@ public: bool Save(const std::string& filename); // Returns true if key exists in section - bool Exists(const std::string& sectionName, const std::string& key) const; + bool Exists(std::string_view section_name, std::string_view key) const; template - bool GetIfExists(const std::string& sectionName, const std::string& key, T* value) + bool GetIfExists(std::string_view section_name, std::string_view key, T* value) { - if (Exists(sectionName, key)) - return GetOrCreateSection(sectionName)->Get(key, value); + if (Exists(section_name, key)) + return GetOrCreateSection(section_name)->Get(key, value); return false; } template - bool GetIfExists(const std::string& sectionName, const std::string& key, T* value, T defaultValue) + bool GetIfExists(std::string_view section_name, std::string_view key, T* value, T default_value) { - if (Exists(sectionName, key)) - return GetOrCreateSection(sectionName)->Get(key, value, defaultValue); - else - *value = defaultValue; + if (Exists(section_name, key)) + return GetOrCreateSection(section_name)->Get(key, value, default_value); + *value = default_value; return false; } - bool GetKeys(const std::string& sectionName, std::vector* keys) const; + bool GetKeys(std::string_view section_name, std::vector* keys) const; - void SetLines(const std::string& sectionName, const std::vector& lines); - void SetLines(const std::string& section_name, std::vector&& lines); - bool GetLines(const std::string& sectionName, std::vector* lines, - const bool remove_comments = true) const; + void SetLines(std::string_view section_name, const std::vector& lines); + void SetLines(std::string_view section_name, std::vector&& lines); + bool GetLines(std::string_view section_name, std::vector* lines, + bool remove_comments = true) const; - bool DeleteKey(const std::string& sectionName, const std::string& key); - bool DeleteSection(const std::string& sectionName); + bool DeleteKey(std::string_view section_name, std::string_view key); + bool DeleteSection(std::string_view section_name); void SortSections(); - Section* GetOrCreateSection(const std::string& section); + Section* GetOrCreateSection(std::string_view section_name); // This function is related to parsing data from lines of INI files // It's used outside of IniFile, which is why it is exposed publicly @@ -167,8 +165,8 @@ public: private: std::list
sections; - const Section* GetSection(const std::string& section) const; - Section* GetSection(const std::string& section); + const Section* GetSection(std::string_view section_name) const; + Section* GetSection(std::string_view section_name); static const std::string& NULL_STRING; };