Browse Source

Additional checks for invalid access to IdentifierStorage

Ivan Savenko 2 years ago
parent
commit
c8a6cd74cc
2 changed files with 19 additions and 11 deletions
  1. 16 8
      lib/modding/IdentifierStorage.cpp
  2. 3 3
      lib/modding/IdentifierStorage.h

+ 16 - 8
lib/modding/IdentifierStorage.cpp

@@ -20,11 +20,6 @@
 
 
 VCMI_LIB_NAMESPACE_BEGIN
 VCMI_LIB_NAMESPACE_BEGIN
 
 
-CIdentifierStorage::CIdentifierStorage():
-	state(LOADING)
-{
-}
-
 void CIdentifierStorage::checkIdentifier(std::string & ID)
 void CIdentifierStorage::checkIdentifier(std::string & ID)
 {
 {
 	if (boost::algorithm::ends_with(ID, "."))
 	if (boost::algorithm::ends_with(ID, "."))
@@ -52,7 +47,7 @@ void CIdentifierStorage::requestIdentifier(ObjectCallback callback) const
 
 
 	assert(!callback.localScope.empty());
 	assert(!callback.localScope.empty());
 
 
-	if (state != FINISHED) // enqueue request if loading is still in progress
+	if (state != ELoadingState::FINISHED) // enqueue request if loading is still in progress
 		scheduledRequests.push_back(callback);
 		scheduledRequests.push_back(callback);
 	else // execute immediately for "late" requests
 	else // execute immediately for "late" requests
 		resolveIdentifier(callback);
 		resolveIdentifier(callback);
@@ -138,6 +133,9 @@ void CIdentifierStorage::tryRequestIdentifier(const std::string & type, const Js
 
 
 std::optional<si32> CIdentifierStorage::getIdentifier(const std::string & scope, const std::string & type, const std::string & name, bool silent) const
 std::optional<si32> CIdentifierStorage::getIdentifier(const std::string & scope, const std::string & type, const std::string & name, bool silent) const
 {
 {
+	//TODO: RE-ENABLE
+	//assert(state != ELoadingState::LOADING);
+
 	auto idList = getPossibleIdentifiers(ObjectCallback::fromNameAndType(scope, type, name, std::function<void(si32)>(), silent));
 	auto idList = getPossibleIdentifiers(ObjectCallback::fromNameAndType(scope, type, name, std::function<void(si32)>(), silent));
 
 
 	if (idList.size() == 1)
 	if (idList.size() == 1)
@@ -150,6 +148,8 @@ std::optional<si32> CIdentifierStorage::getIdentifier(const std::string & scope,
 
 
 std::optional<si32> CIdentifierStorage::getIdentifier(const std::string & type, const JsonNode & name, bool silent) const
 std::optional<si32> CIdentifierStorage::getIdentifier(const std::string & type, const JsonNode & name, bool silent) const
 {
 {
+	assert(state != ELoadingState::LOADING);
+
 	auto idList = getPossibleIdentifiers(ObjectCallback::fromNameAndType(name.meta, type, name.String(), std::function<void(si32)>(), silent));
 	auto idList = getPossibleIdentifiers(ObjectCallback::fromNameAndType(name.meta, type, name.String(), std::function<void(si32)>(), silent));
 
 
 	if (idList.size() == 1)
 	if (idList.size() == 1)
@@ -162,6 +162,8 @@ std::optional<si32> CIdentifierStorage::getIdentifier(const std::string & type,
 
 
 std::optional<si32> CIdentifierStorage::getIdentifier(const JsonNode & name, bool silent) const
 std::optional<si32> CIdentifierStorage::getIdentifier(const JsonNode & name, bool silent) const
 {
 {
+	assert(state != ELoadingState::LOADING);
+
 	auto idList = getPossibleIdentifiers(ObjectCallback::fromNameWithType(name.meta, name.String(), std::function<void(si32)>(), silent));
 	auto idList = getPossibleIdentifiers(ObjectCallback::fromNameWithType(name.meta, name.String(), std::function<void(si32)>(), silent));
 
 
 	if (idList.size() == 1)
 	if (idList.size() == 1)
@@ -174,6 +176,8 @@ std::optional<si32> CIdentifierStorage::getIdentifier(const JsonNode & name, boo
 
 
 std::optional<si32> CIdentifierStorage::getIdentifier(const std::string & scope, const std::string & fullName, bool silent) const
 std::optional<si32> CIdentifierStorage::getIdentifier(const std::string & scope, const std::string & fullName, bool silent) const
 {
 {
+	assert(state != ELoadingState::LOADING);
+
 	auto idList = getPossibleIdentifiers(ObjectCallback::fromNameWithType(scope, fullName, std::function<void(si32)>(), silent));
 	auto idList = getPossibleIdentifiers(ObjectCallback::fromNameWithType(scope, fullName, std::function<void(si32)>(), silent));
 
 
 	if (idList.size() == 1)
 	if (idList.size() == 1)
@@ -186,6 +190,8 @@ std::optional<si32> CIdentifierStorage::getIdentifier(const std::string & scope,
 
 
 void CIdentifierStorage::registerObject(const std::string & scope, const std::string & type, const std::string & name, si32 identifier)
 void CIdentifierStorage::registerObject(const std::string & scope, const std::string & type, const std::string & name, si32 identifier)
 {
 {
+	assert(state != ELoadingState::FINISHED);
+
 	ObjectData data;
 	ObjectData data;
 	data.scope = scope;
 	data.scope = scope;
 	data.id = identifier;
 	data.id = identifier;
@@ -311,7 +317,9 @@ bool CIdentifierStorage::resolveIdentifier(const ObjectCallback & request) const
 
 
 void CIdentifierStorage::finalize()
 void CIdentifierStorage::finalize()
 {
 {
-	state = FINALIZING;
+	assert(state == ELoadingState::LOADING);
+
+	state = ELoadingState::FINALIZING;
 	bool errorsFound = false;
 	bool errorsFound = false;
 
 
 	while ( !scheduledRequests.empty() )
 	while ( !scheduledRequests.empty() )
@@ -333,7 +341,7 @@ void CIdentifierStorage::finalize()
 		logMod->error("All known identifiers were dumped into log file");
 		logMod->error("All known identifiers were dumped into log file");
 	}
 	}
 	assert(errorsFound == false);
 	assert(errorsFound == false);
-	state = FINISHED;
+	state = ELoadingState::FINISHED;
 }
 }
 
 
 VCMI_LIB_NAMESPACE_END
 VCMI_LIB_NAMESPACE_END

+ 3 - 3
lib/modding/IdentifierStorage.h

@@ -17,7 +17,7 @@ class JsonNode;
 /// if possible, objects ID's should be in format <type>.<name>, camelCase e.g. "creature.grandElf"
 /// if possible, objects ID's should be in format <type>.<name>, camelCase e.g. "creature.grandElf"
 class DLL_LINKAGE CIdentifierStorage
 class DLL_LINKAGE CIdentifierStorage
 {
 {
-	enum ELoadingState
+	enum class ELoadingState
 	{
 	{
 		LOADING,
 		LOADING,
 		FINALIZING,
 		FINALIZING,
@@ -63,7 +63,7 @@ class DLL_LINKAGE CIdentifierStorage
 	std::multimap<std::string, ObjectData> registeredObjects;
 	std::multimap<std::string, ObjectData> registeredObjects;
 	mutable std::vector<ObjectCallback> scheduledRequests;
 	mutable std::vector<ObjectCallback> scheduledRequests;
 
 
-	ELoadingState state;
+	ELoadingState state = ELoadingState::LOADING;
 
 
 	/// Check if identifier can be valid (camelCase, point as separator)
 	/// Check if identifier can be valid (camelCase, point as separator)
 	static void checkIdentifier(std::string & ID);
 	static void checkIdentifier(std::string & ID);
@@ -73,7 +73,7 @@ class DLL_LINKAGE CIdentifierStorage
 	std::vector<ObjectData> getPossibleIdentifiers(const ObjectCallback & callback) const;
 	std::vector<ObjectData> getPossibleIdentifiers(const ObjectCallback & callback) const;
 
 
 public:
 public:
-	CIdentifierStorage();
+	CIdentifierStorage() = default;
 	virtual ~CIdentifierStorage() = default;
 	virtual ~CIdentifierStorage() = default;
 
 
 	/// request identifier for specific object name.
 	/// request identifier for specific object name.