Make ConfigDB properly thread-safe.

Replace CreateValue with SetValueString so we don't pass pointers to
internal
 ConfigDB structures around. Refs #1810.
Remove some splash screen related functions since we can achieve the
same
 with the ConfigDB functions exposed to scripts.

This was SVN commit r14437.

binaries/data/mods/public/gui/pregame/mainmenu.js

@@ -158,7 +158,7 @@ function onTick()
 	{
 		g_ShowSplashScreens = false;
 
-		if (Engine.IsSplashScreenEnabled())
+		if (Engine.ConfigDB_GetValue("user", "splashscreenenable") !== "false")
 			Engine.PushGuiPage("page_splashscreen.xml", { "page": "splashscreen" } );
 
 		// Warn about removing fixed render path

binaries/data/mods/public/gui/splashscreen/splashscreen.xml

@@ -22,7 +22,8 @@
 		<object name="btnOK" type="button" style="StoneButton" tooltip_style="snToolTip" size="24 100%-52 188 100%-24">
 			OK
 			<action on="Press"><![CDATA[
-			Engine.SetSplashScreenEnabled(getGUIObjectByName("displaySplashScreen").checked);
+			Engine.ConfigDB_CreateValue("user", "splashscreenenable", getGUIObjectByName("displaySplashScreen").checked ? "true" : "false");
+			Engine.ConfigDB_WriteFile("user", "config/user.cfg");
 			Engine.PopGuiPage();
 			]]></action>
 		</object>

source/gui/scripting/ScriptFunctions.cpp

@@ -504,21 +504,6 @@ bool IsUserReportEnabled(void* UNUSED(cbdata))
 	return g_UserReporter.IsReportingEnabled();
 }
 
-bool IsSplashScreenEnabled(void* UNUSED(cbdata))
-{
-	bool splashScreenEnable = true;
-	CFG_GET_VAL("splashscreenenable", Bool, splashScreenEnable);
-	return splashScreenEnable;
-}
-
-void SetSplashScreenEnabled(void* UNUSED(cbdata), bool enabled)
-{
-	CStr val = (enabled ? "true" : "false");
-	g_ConfigDB.CreateValue(CFG_USER, "splashscreenenable")->m_String = val;
-	g_ConfigDB.WriteFile(CFG_USER);
-}
-
-
 void SetUserReportEnabled(void* UNUSED(cbdata), bool enabled)
 {
 	g_UserReporter.SetReportingEnabled(enabled);
@@ -701,10 +686,6 @@ void GuiScriptingInit(ScriptInterface& scriptInterface)
 	scriptInterface.RegisterFunction<std::string, &GetUserReportStatus>("GetUserReportStatus");
 	scriptInterface.RegisterFunction<void, std::string, int, std::wstring, &SubmitUserReport>("SubmitUserReport");
 
-	// Splash screen functions
-	scriptInterface.RegisterFunction<bool, &IsSplashScreenEnabled>("IsSplashScreenEnabled");
-	scriptInterface.RegisterFunction<void, bool, &SetSplashScreenEnabled>("SetSplashScreenEnabled");
-
 	// Development/debugging functions
 	scriptInterface.RegisterFunction<void, float, &SetSimRate>("SetSimRate");
 	scriptInterface.RegisterFunction<float, &GetSimRate>("GetSimRate");

source/network/NetServer.cpp

@@ -221,16 +221,16 @@ void* CNetServerWorker::SetupUPnP(void*)
 	struct UPNPDev* devlist = 0;
 
 	// Cached root descriptor URL.
-	std::string rootDescURL = "";
+	std::string rootDescURL;
 	CFG_GET_VAL("network.upnprootdescurl", String, rootDescURL);
-	if (rootDescURL != "")
+	if (!rootDescURL.empty())
 		LOGMESSAGE(L"Net server: attempting to use cached root descriptor URL: %hs", rootDescURL.c_str());
 
 	// Init the return variable for UPNP_GetValidIGD to 1 so things behave when using cached URLs.
 	int ret = 1;
 
 	// If we have a cached URL, try that first, otherwise try getting a valid UPnP device for 10 seconds. We also get our LAN address here.
-	if (!((rootDescURL != "" && UPNP_GetIGDFromUrl(rootDescURL.c_str(), &urls, &data, internalIPAddress, sizeof(internalIPAddress)))
+	if (!((!rootDescURL.empty() && UPNP_GetIGDFromUrl(rootDescURL.c_str(), &urls, &data, internalIPAddress, sizeof(internalIPAddress)))
 	  || ((devlist = upnpDiscover(10000, 0, 0, 0, 0, 0)) && (ret = UPNP_GetValidIGD(devlist, &urls, &data, internalIPAddress, sizeof(internalIPAddress))))))
 	{
 		LOGMESSAGE(L"Net server: upnpDiscover failed and no working cached URL.");
@@ -288,7 +288,7 @@ void* CNetServerWorker::SetupUPnP(void*)
 				   externalIPAddress, psPort, protocall, intClient, intPort, duration);
 
 	// Cache root descriptor URL to try to avoid discovery next time.
-	g_ConfigDB.CreateValue(CFG_USER, "network.upnprootdescurl")->m_String = urls.controlURL;
+	g_ConfigDB.SetValueString(CFG_USER, "network.upnprootdescurl", urls.controlURL);
 	g_ConfigDB.WriteFile(CFG_USER);
 	LOGMESSAGE(L"Net server: cached UPnP root descriptor URL as %hs", urls.controlURL);
 

source/ps/ConfigDB.cpp

@@ -1,4 +1,4 @@
-/* Copyright (C) 2012 Wildfire Games.
+/* Copyright (C) 2013 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -39,6 +39,7 @@ struct ScopedLock
 
 CConfigDB::CConfigDB()
 {
+	// Recursive mutex needed for WriteFile
 	pthread_mutexattr_t attr;
 	int err;
 	err = pthread_mutexattr_init(&attr);
@@ -51,36 +52,66 @@ CConfigDB::CConfigDB()
 	ENSURE(err == 0);
 }
 
-CConfigValue *CConfigDB::GetValue(EConfigNamespace ns, const CStr& name)
+#define GETVAL(T, type) \
-{
+	void CConfigDB::GetValue##T(EConfigNamespace ns, const CStr& name, type& value) \
-	ScopedLock s;
+	{ \
-	CConfigValueSet* values = GetValues(ns, name);
+		if (ns < 0 || ns >= CFG_LAST) \
-	if (!values)
+		{ \
-		return NULL;
+			debug_warn(L"CConfigDB: Invalid ns value"); \
-	return &((*values)[0]);
+			return; \
-}
+		} \
+		ScopedLock s; \
+		TConfigMap::iterator it = m_Map[CFG_COMMAND].find(name); \
+		if (it != m_Map[CFG_COMMAND].end()) \
+		{ \
+			it->second[0].Get##T(value); \
+			return; \
+		} \
+		\
+		for (int search_ns = ns; search_ns >= 0; search_ns--) \
+		{ \
+			it = m_Map[search_ns].find(name); \
+			if (it != m_Map[search_ns].end()) \
+			{ \
+				it->second[0].Get##T(value); \
+				return; \
+			} \
+		} \
+	}
 
-CConfigValueSet *CConfigDB::GetValues(EConfigNamespace ns, const CStr& name)
+GETVAL(Bool, bool)
+GETVAL(Int, int)
+GETVAL(Float, float)
+GETVAL(Double, double)
+GETVAL(String, std::string)
+
+#undef GETVAL
+
+void CConfigDB::GetValues(EConfigNamespace ns, const CStr& name, CConfigValueSet& values)
 {
 	if (ns < 0 || ns >= CFG_LAST)
 	{
 		debug_warn(L"CConfigDB: Invalid ns value");
-		return NULL;
+		return;
 	}
 
 	ScopedLock s;
 	TConfigMap::iterator it = m_Map[CFG_COMMAND].find(name);
 	if (it != m_Map[CFG_COMMAND].end())
-		return &(it->second);
+	{
+		values = it->second;
+		return;
+	}
 
 	for (int search_ns = ns; search_ns >= 0; search_ns--)
 	{
-		TConfigMap::iterator it = m_Map[search_ns].find(name);
+		it = m_Map[search_ns].find(name);
 		if (it != m_Map[search_ns].end())
-			return &(it->second);
+		{
+			values = it->second;
+			return;
+		}
 	}
-
-	return NULL;
 }
 
 EConfigNamespace CConfigDB::GetValueNamespace(EConfigNamespace ns, const CStr& name)
@@ -98,7 +129,7 @@ EConfigNamespace CConfigDB::GetValueNamespace(EConfigNamespace ns, const CStr& n
 
 	for (int search_ns = ns; search_ns >= 0; search_ns--)
 	{
-		TConfigMap::iterator it = m_Map[search_ns].find(name);
+		it = m_Map[search_ns].find(name);
 		if (it != m_Map[search_ns].end())
 			return (EConfigNamespace)search_ns;
 	}
@@ -128,24 +159,29 @@ std::map<CStr, CConfigValueSet> CConfigDB::GetValuesWithPrefix(EConfigNamespace
 		}
 	}
 
+	for (TConfigMap::iterator it = m_Map[CFG_COMMAND].begin(); it != m_Map[CFG_COMMAND].end(); ++it)
+	{
+		if (boost::algorithm::starts_with(it->first, prefix))
+			ret[it->first] = it->second;
+	}
+
 	return ret;
 }
 
-CConfigValue *CConfigDB::CreateValue(EConfigNamespace ns, const CStr& name)
+void CConfigDB::SetValueString(EConfigNamespace ns, const CStr& name, const CStr& value)
 {
 	if (ns < 0 || ns >= CFG_LAST)
 	{
 		debug_warn(L"CConfigDB: Invalid ns value");
-		return NULL;
+		return;
 	}
 
 	ScopedLock s;
 	TConfigMap::iterator it = m_Map[ns].find(name);
-	if (it != m_Map[ns].end())
+	if (it == m_Map[ns].end())
-		return &(it->second[0]);
+		it = m_Map[ns].insert(m_Map[ns].begin(), make_pair(name, CConfigValueSet(1)));
 
-	it=m_Map[ns].insert(m_Map[ns].begin(), make_pair(name, CConfigValueSet(1)));
+	it->second[0].m_String = value;
-	return &(it->second[0]);
 }
 
 void CConfigDB::SetConfigFile(EConfigNamespace ns, const VfsPath& path)

source/ps/ConfigDB.h

@@ -60,23 +60,26 @@ public:
 	CConfigDB();
 
 	/**
-	 * Attempt to find a config variable with the given name; will search
+	 * Attempt to retrieve the value of a config variable with the given name;
-	 * CFG_COMMAND first, and then all namespaces from the specified namespace
+	 * will search CFG_COMMAND first, and then all namespaces from the specified
-	 * down to system.
+	 * namespace down.
-	 *
-	 * Returns a pointer to the config value structure for the variable, or
-	 * NULL if such a variable could not be found.
 	 */
-	CConfigValue *GetValue(EConfigNamespace ns, const CStr& name);
+	void GetValueBool(EConfigNamespace ns, const CStr& name, bool& value);
+	///@copydoc CConfigDB::GetValueBool
+	void GetValueInt(EConfigNamespace ns, const CStr& name, int& value);
+	///@copydoc CConfigDB::GetValueBool
+	void GetValueFloat(EConfigNamespace ns, const CStr& name, float& value);
+	///@copydoc CConfigDB::GetValueBool
+	void GetValueDouble(EConfigNamespace ns, const CStr& name, double& value);
+	///@copydoc CConfigDB::GetValueBool
+	void GetValueString(EConfigNamespace ns, const CStr& name, std::string& value);
 
 	/**
 	 * Attempt to retrieve a vector of values corresponding to the given setting;
 	 * will search CFG_COMMAND first, and then all namespaces from the specified
-	 * namespace down to system.
+	 * namespace down.
-	 *
-	 * Returns a pointer to the vector, or NULL if the setting could not be found.
 	 */
-	CConfigValueSet *GetValues(EConfigNamespace ns, const CStr& name);
+	void GetValues(EConfigNamespace ns, const CStr& name, CConfigValueSet& values);
 
 	/**
 	 * Returns the namespace that the value returned by GetValues was defined in,
@@ -87,18 +90,15 @@ public:
 	/**
 	 * Retrieve a map of values corresponding to settings whose names begin
 	 * with the given prefix;
-	 * will search all namespaces from system up to the specified namespace.
+	 * will search all namespaces from default up to the specified namespace.
 	 */
 	std::map<CStr, CConfigValueSet> GetValuesWithPrefix(EConfigNamespace ns, const CStr& prefix);
 
 	/**
-	 * Create a new config value in the specified namespace. If such a
+	 * Save a config value in the specified namespace. If the config variable
-	 * variable already exists in this namespace, the old value is returned.
+	 * existed the value is replaced.
-	 *
-	 * Returns a pointer to the value of the newly created config variable, or
-	 * that of the already existing config variable.
 	 */
-	CConfigValue *CreateValue(EConfigNamespace ns, const CStr& name);
+	void SetValueString(EConfigNamespace ns, const CStr& name, const CStr& value);
 	
 	/**
 	 * Set the path to the config file used to populate the specified namespace
@@ -145,11 +145,6 @@ public:
 // convenience wrapper on top of CConfigValue::Get* simplifies user code and
 // avoids "assignment within condition expression" warnings.
 #define CFG_GET_VAL(name, type, destination)\
-STMT(\
+	g_ConfigDB.GetValue##type(CFG_USER, name, destination)
-	CConfigValue* val = g_ConfigDB.GetValue(CFG_USER, name);\
-	if(val)\
-		val->Get##type(destination);\
-)
-
 
 #endif

source/ps/GameSetup/Config.cpp

@@ -152,7 +152,7 @@ static void ProcessCommandLineArgs(const CmdLineArgs& args)
 		{
 			CStr name = name_value.BeforeFirst(":");
 			CStr value = name_value.AfterFirst(":");
-			g_ConfigDB.CreateValue(CFG_COMMAND, name)->m_String = value;
+			g_ConfigDB.SetValueString(CFG_COMMAND, name, value);
 		}
 	}
 
@@ -167,7 +167,7 @@ static void ProcessCommandLineArgs(const CmdLineArgs& args)
 //		trace_enable(true);
 
 	if (args.Has("profile"))
-		g_ConfigDB.CreateValue(CFG_COMMAND, "profile")->m_String = args.Get("profile");
+		g_ConfigDB.SetValueString(CFG_COMMAND, "profile", args.Get("profile"));
 
 	if (args.Has("quickstart"))
 	{
@@ -179,22 +179,22 @@ static void ProcessCommandLineArgs(const CmdLineArgs& args)
 		g_DisableAudio = true;
 
 	if (args.Has("shadows"))
-		g_ConfigDB.CreateValue(CFG_COMMAND, "shadows")->m_String = "true";
+		g_ConfigDB.SetValueString(CFG_COMMAND, "shadows", "true");
 
 	if (args.Has("xres"))
-		g_ConfigDB.CreateValue(CFG_COMMAND, "xres")->m_String = args.Get("xres");
+		g_ConfigDB.SetValueString(CFG_COMMAND, "xres", args.Get("xres"));
 
 	if (args.Has("yres"))
-		g_ConfigDB.CreateValue(CFG_COMMAND, "yres")->m_String = args.Get("yres");
+		g_ConfigDB.SetValueString(CFG_COMMAND, "yres", args.Get("yres"));
 
 	if (args.Has("vsync"))
-		g_ConfigDB.CreateValue(CFG_COMMAND, "vsync")->m_String = "true";
+		g_ConfigDB.SetValueString(CFG_COMMAND, "vsync", "true");
 
 	if (args.Has("ooslog"))
-		g_ConfigDB.CreateValue(CFG_COMMAND, "ooslog")->m_String = "true";
+		g_ConfigDB.SetValueString(CFG_COMMAND, "ooslog", "true");
 
 	if (args.Has("serializationtest"))
-		g_ConfigDB.CreateValue(CFG_COMMAND, "serializationtest")->m_String = "true";
+		g_ConfigDB.SetValueString(CFG_COMMAND, "serializationtest", "true");
 }
 
 

source/ps/UserReport.cpp

@@ -1,4 +1,4 @@
-/* Copyright (C) 2012 Wildfire Games.
+/* Copyright (C) 2013 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -537,7 +537,7 @@ std::string CUserReporter::LoadUserID()
 			userID += hex;
 		}
 
-		g_ConfigDB.CreateValue(CFG_USER, "userreport.id")->m_String = userID;
+		g_ConfigDB.SetValueString(CFG_USER, "userreport.id", userID);
 		g_ConfigDB.WriteFile(CFG_USER);
 	}
 
@@ -554,7 +554,7 @@ bool CUserReporter::IsReportingEnabled()
 void CUserReporter::SetReportingEnabled(bool enabled)
 {
 	CStr val = CStr::FromInt(enabled ? REPORTER_VERSION : 0);
-	g_ConfigDB.CreateValue(CFG_USER, "userreport.enabledversion")->m_String = val;
+	g_ConfigDB.SetValueString(CFG_USER, "userreport.enabledversion", val);
 	g_ConfigDB.WriteFile(CFG_USER);
 
 	if (m_Worker)

source/ps/scripting/JSInterface_ConfigDB.cpp

@@ -48,16 +48,9 @@ std::string JSI_ConfigDB::GetValue(void* UNUSED(cbdata), std::wstring cfgNsStrin
 	if (!GetConfigNamespace(cfgNsString, cfgNs))
 		return std::string();
 
-	CConfigValue *val = g_ConfigDB.GetValue(cfgNs, name);
+	std::string value;
-	if (val)
+	g_ConfigDB.GetValueString(cfgNs, name, value);
-	{
+	return value;
-		return val->m_String;
-	}
-	else
-	{
-		LOGMESSAGE(L"Config setting %hs does not exist!", name.c_str());
-		return std::string();
-	}
 }
 
 bool JSI_ConfigDB::CreateValue(void* UNUSED(cbdata), std::wstring cfgNsString, std::string name, std::string value)
@@ -66,8 +59,7 @@ bool JSI_ConfigDB::CreateValue(void* UNUSED(cbdata), std::wstring cfgNsString, s
 	if (!GetConfigNamespace(cfgNsString, cfgNs))
 		return false;
 
-	CConfigValue *val = g_ConfigDB.CreateValue(cfgNs, name);
+	g_ConfigDB.SetValueString(cfgNs, name, value);
-	val->m_String = value;
 	return true;
 }