Tightened up file handling code for config stores so that Windows is happy.

src/config.cpp

@@ -38,8 +38,8 @@ extern "C" {
 #include <libguess.h>
 }
 #include <iconv.h>
-#include <errno.h>
 #endif
+#include <errno.h>
 
 /* http://stackoverflow.com/a/1031773 */
 static bool validUtf8(const char *string)
@@ -159,121 +159,143 @@ Config::Config()
 	midi.chorus = false;
 	midi.reverb = false;
 	SE.sourceCount = 6;
+	execPath = std::string("");
 }
 
 /* Pre-convert value to string since we don't care about the type at this point anymore */
-static bool updateConfigFileValue(const char *key, const char *value)
+bool Config::updateConfigFileValue(const char *key, const char *value)
 {
-	/* Open files for reading and writing */
+	std::string confPath = execPath + CONF_FILE;
-	std::ifstream confRead;
+	std::string tempPath = execPath + CONF_FILE_TMP;
-	confRead.open(CONF_FILE);
+	/* Insert scope so that file handles definitely get cleaned up, even on bitchy windows */
-	std::ofstream confWrite;
-	confWrite.open(CONF_FILE_TMP, std::ofstream::trunc);
-
-	if(confWrite && confRead)
 	{
-		/* Buffer to store config file lines */
+		/* Open files for reading and writing */
-		char buf[512];
+		std::ifstream confRead;
-		bool valWritten = false;
+		confRead.open(confPath.c_str());
-		while(!confRead.eof())
+		std::ofstream confWrite;
+		confWrite.open(tempPath.c_str(), std::ofstream::trunc);
+
+		if(confWrite && confWrite.is_open() && confRead && confRead.is_open())
 		{
-			/* Get next line */
+			/* Buffer to store config file lines */
-			confRead.getline(buf, 512);
+			char buf[512];
+			bool valWritten = false;
+			while(!confRead.eof())
+			{
+				/* Get next line */
+				confRead.getline(buf, 512);
 
-			if(confRead.fail())
+				if(confRead.fail())
-			{
+				{
-				/* This means we either encountered an empty line,
+					/* This means we either encountered an empty line,
-				/* which is fine, or the line was too big. Maybe
+					/* which is fine, or the line was too big. Maybe
-				/* handle the case of long lines in the future? */
+					/* handle the case of long lines in the future? */
-			}
+				}
-			else if(strncmp(buf, key, strlen(key)) == 0)
+				else if(strncmp(buf, key, strlen(key)) == 0)
-			{
+				{
-				/* This is the line we want to overwrite. For now
+					/* This is the line we want to overwrite. For now
-				/* discard duplicate lines of the same key */
+					/* discard duplicate lines of the same key */
-				if(valWritten)
+					if(valWritten)
-					continue;
+						continue;
 
-				confWrite << key << '=' << value;
+					confWrite << key << '=' << value;
-				valWritten = true;
+					valWritten = true;
+				}
+				else
+				{
+					/* Just copy the old line into the new file,
+					/* use gcount()-1 to not copy null character
+					/* but only if not EOF */
+					assert(confRead.gcount() > 0 && confRead.gcount() < 512);
+					int trim = confRead.eof() ? 0 : 1;
+					confWrite.write(buf, confRead.gcount()-trim);
+				}
+				if(!confRead.eof())
+					confWrite << '\n';
 			}
-			else
+
-			{
+			confRead.close();
-				/* Just copy the old line into the new file,
+			confWrite.close();
-				/* use gcount()-1 to not copy null character
-				/* but only if not EOF */
-				assert(confRead.gcount() > 0 && confRead.gcount() < 512);
-				int trim = confRead.eof() ? 0 : 1;
-				confWrite.write(buf, confRead.gcount()-trim);
-			}
-			if(!confRead.eof())
-				confWrite << '\n';
 		}
+		else
+		{
+			Debug() << CONF_FILE": Failed to update config file.";
 
-		confRead.close();
+			if(confRead.is_open())
-		confWrite.close();
+				confRead.close();
 
-		/* Backup old config file and move the new one into place */
+			if(confWrite.is_open())
-		remove(CONF_FILE);
+				confWrite.close();
-		rename(CONF_FILE_TMP, CONF_FILE);
 
-		return true;
+			return false;
+		}
 	}
-	Debug() << CONF_FILE": Failed to update config file.\n";
 
-	if(confRead.is_open())
+	/* Backup old config file and move the new one into place */
-		confRead.close();
+	if(std::remove(confPath.c_str()) == -1){
+		Debug() << CONF_FILE": removal of the old config failed.";
+	}
+	if(std::rename(tempPath.c_str(), confPath.c_str()) != 0){
+		Debug() << CONF_FILE": replacing the old config failed.";
+	}
 
-	if(confWrite.is_open())
+	return true;
-		confWrite.close();
-
-	return false;
 }
 
 bool Config::store(const char *key, int value)
 {
-	std::ifstream confFileRead;
+	bool update = false;
-	confFileRead.open(CONF_FILE);
+	std::string confPath = execPath + CONF_FILE;
-	int curValue;
+	/* Insert scope to make sure file handles get cleaned up */
-	if(confFileRead)
 	{
-		/* First read the config file to get the currently stored value
+		std::ifstream confFileRead;
-		/* and check whether it's in the config file at all */
+		confFileRead.open(confPath.c_str());
-		po::options_description podesc;
+		int curValue;
-		po::variables_map vm;
+		if(confFileRead && confFileRead.is_open())
-		podesc.add_options()(key, po::value<int>()->required());
-		try
 		{
+			/* First read the config file to get the currently stored value
+			/* and check whether it's in the config file at all */
+			po::options_description podesc;
+			po::variables_map vm;
+			podesc.add_options()(key, po::value<int>()->required());
 			try
 			{
-				po::store(po::parse_config_file(confFileRead, podesc, true), vm);
+				try
-				po::notify(vm);
+				{
-				curValue = vm[key].as<int>();
+					po::store(po::parse_config_file(confFileRead, podesc, true), vm);
-			}
+					po::notify(vm);
-			catch(po::multiple_occurrences e)
+					curValue = vm[key].as<int>();
-			{
+				}
-				/* This is fine we're gonna remove the duplicates when storing */
+				catch(po::multiple_occurrences e)
-				curValue = ~value;
+				{
-			}
+					/* This is fine we're gonna remove the duplicates when storing */
+					curValue = ~value;
+				}
 
-			if(curValue == value)
+				if(curValue == value)
-			{
+				{
-				/* We don't need to store anything */
+					/* We don't need to store anything */
-				return true;
+					confFileRead.close();
-			}
+					return true;
-			else
+				}
-			{
+				/* needs file update */
-				/* Update the relevant config file line */
+				update = true;
-				char num[21];
+			} catch (...) {}
-				snprintf(num, 21, "%d", value);
+			confFileRead.close();
-				return updateConfigFileValue(key, num);
+		}
-			}
+	}
-		} catch (...) {}
+
-		confFileRead.close();
+	if(update)
+	{
+		/* Update the relevant config file line */
+		char num[21];
+		snprintf(num, 21, "%d", value);
+		return updateConfigFileValue(key, num);
 	}
 
 	/* Open file for writing */
 	std::ofstream confFileWrite(CONF_FILE, std::ofstream::app);
-	if(confFileWrite)
+	if(confFileWrite && confFileWrite.is_open())
 	{
 		/* Append new config line */
 		confFileWrite << '\n' << key << '=' << value;
@@ -288,45 +310,56 @@ bool Config::store(const char *key, int value)
 
 bool Config::store(const char *key, bool value)
 {
-	std::ifstream confFileRead;
+	bool update = false;
-	confFileRead.open(CONF_FILE);
+	std::string confPath = execPath + CONF_FILE;
-	bool curValue;
+	/* Insert scope to make sure file handles get cleaned up */
-	if(confFileRead)
 	{
-		/* First read the config file to get the currently stored value */
+		std::ifstream confFileRead;
-		po::options_description podesc;
+		confFileRead.open(confPath.c_str());
-		po::variables_map vm;
+		bool curValue;
-		podesc.add_options()(key, po::value<bool>()->required());
+		if(confFileRead && confFileRead.is_open())
-		try
 		{
+			/* First read the config file to get the currently stored value
+			/* and check whether it's in the config file at all */
+			po::options_description podesc;
+			po::variables_map vm;
+			podesc.add_options()(key, po::value<bool>()->required());
 			try
 			{
-				po::store(po::parse_config_file(confFileRead, podesc, true), vm);
+				try
-				po::notify(vm);
+				{
-				curValue = vm[key].as<bool>();
+					po::store(po::parse_config_file(confFileRead, podesc, true), vm);
-			}
+					po::notify(vm);
-			catch(po::multiple_occurrences &e)
+					curValue = vm[key].as<bool>();
-			{
+				}
-				/* This is fine we're gonna remove the duplicates when storing */
+				catch(po::multiple_occurrences e)
-				curValue = !value;
+				{
-			}
+					/* This is fine we're gonna remove the duplicates when storing */
-			if(curValue == value)
+					curValue = ~value;
-			{
+				}
-				/* We don't need to store anything */
+
-				return true;
+				if(curValue == value)
-			}
+				{
-			else
+					/* We don't need to store anything */
-			{
+					confFileRead.close();
-				/* Update the relevant config file line */
+					return true;
-				return updateConfigFileValue(key, value ? "true" : "false");
+				}
-			}
+				/* needs file update */
-		} catch (...) {}
+				update = true;
-		confFileRead.close();
+			} catch (...) {}
+			confFileRead.close();
+		}
+	}
+
+	if(update)
+	{
+		/* Update the relevant config file line */
+		return updateConfigFileValue(key, value ? "true" : "false");
 	}
 
 	/* Open file for writing */
 	std::ofstream confFileWrite(CONF_FILE, std::ofstream::app);
-	if(confFileWrite)
+	if(confFileWrite && confFileWrite.is_open())
 	{
 		/* Append new config line */
 		confFileWrite << '\n' << key << '=' << std::boolalpha << value;

src/config.h

@@ -90,6 +90,7 @@ struct Config
 	/* Internal */
 	std::string customDataPath;
 	std::string commonDataPath;
+	std::string execPath;
 
 	Config();
 
@@ -97,6 +98,9 @@ struct Config
 	bool store(const char* key, int value);
 	bool store(const char* key, bool value);
 	void readGameINI();
+
+	private:
+	bool updateConfigFileValue(const char *key, const char *value);
 };
 
 #endif // CONFIG_H

src/main.cpp

@@ -205,6 +205,13 @@ int main(int argc, char *argv[])
 	/* now we load the config */
 	Config conf;
 
+	char *execDir = SDL_GetBasePath();
+	if (execDir)
+	{
+		conf.execPath = std::string(execDir);
+		SDL_free(execDir);
+	}
+
 	conf.read(argc, argv);
 	conf.readGameINI();