Use const references for Settings methods

Also check for (this == &other) before locking mutexes.

3 files changed, 71 insertions, 78 deletions

diff --git a/src/settings.h b/src/settings.h
index a1839b78a..6300ddd6f 100644
--- a/src/settings.h
+++ b/src/settings.h
@@ -63,11 +63,11 @@ public:
 	{
 	}
 
-	void writeLines(std::ostream &os)
+	void writeLines(std::ostream &os) const
 	{
 		JMutexAutoLock lock(m_mutex);
 
-		for(std::map<std::string, std::string>::iterator
+		for(std::map<std::string, std::string>::const_iterator
 				i = m_settings.begin();
 				i != m_settings.end(); ++i)
 		{
@@ -78,9 +78,10 @@ public:
 	}
   
 	// return all keys used
-	std::vector<std::string> getNames(){
+	std::vector<std::string> getNames() const
+	{
 		std::vector<std::string> names;
-		for(std::map<std::string, std::string>::iterator
+		for(std::map<std::string, std::string>::const_iterator
 				i = m_settings.begin();
 				i != m_settings.end(); ++i)
 		{
@@ -90,7 +91,7 @@ public:
 	}
 
 	// remove a setting
-	bool remove(const std::string& name)
+	bool remove(const std::string &name)
 	{
 		return m_settings.erase(name);
 	}
@@ -289,7 +290,7 @@ public:
 		// If something not yet determined to have been changed, check if
 		// any new stuff was added
 		if(!something_actually_changed){
-			for(std::map<std::string, std::string>::iterator
+			for(std::map<std::string, std::string>::const_iterator
 					i = m_settings.begin();
 					i != m_settings.end(); ++i)
 			{
@@ -314,7 +315,7 @@ public:
 			/*
 				Write updated stuff
 			*/
-			for(std::list<std::string>::iterator
+			for(std::list<std::string>::const_iterator
 					i = objects.begin();
 					i != objects.end(); ++i)
 			{
@@ -324,7 +325,7 @@ public:
 			/*
 				Write stuff that was not already in the file
 			*/
-			for(std::map<std::string, std::string>::iterator
+			for(std::map<std::string, std::string>::const_iterator
 					i = m_settings.begin();
 					i != m_settings.end(); ++i)
 			{
@@ -421,14 +422,14 @@ public:
 		return true;
 	}
 
-	void set(std::string name, std::string value)
+	void set(const std::string &name, std::string value)
 	{
 		JMutexAutoLock lock(m_mutex);
 
 		m_settings[name] = value;
 	}
 
-	void set(std::string name, const char *value)
+	void set(const std::string &name, const char *value)
 	{
 		JMutexAutoLock lock(m_mutex);
 
@@ -436,22 +437,22 @@ public:
 	}
 
 
-	void setDefault(std::string name, std::string value)
+	void setDefault(const std::string &name, std::string value)
 	{
 		JMutexAutoLock lock(m_mutex);
 
 		m_defaults[name] = value;
 	}
 
-	bool exists(std::string name) const
+	bool exists(const std::string &name) const
 	{
 		JMutexAutoLock lock(m_mutex);
 
-		return m_settings.find(name) != m_settings.end() 
-				|| m_defaults.find(name) != m_defaults.end();
+		return (m_settings.find(name) != m_settings.end() ||
+			m_defaults.find(name) != m_defaults.end());
 	}
 
-	std::string get(std::string name) const
+	std::string get(const std::string &name) const
 	{
 		JMutexAutoLock lock(m_mutex);
 
@@ -464,12 +465,12 @@ public:
 	}
 
 	//////////// Get setting
-	bool getBool(std::string name) const
+	bool getBool(const std::string &name) const
 	{
 		return is_yes(get(name));
 	}
 
-	bool getFlag(std::string name) const
+	bool getFlag(const std::string &name) const
 	{
 		try {
 			return getBool(name);
@@ -478,27 +479,27 @@ public:
 		}
 	}
 
-	float getFloat(std::string name) const
+	float getFloat(const std::string &name) const
 	{
 		return stof(get(name));
 	}
 
-	u16 getU16(std::string name) const
+	u16 getU16(const std::string &name) const
 	{
 		return stoi(get(name), 0, 65535);
 	}
 
-	s16 getS16(std::string name) const
+	s16 getS16(const std::string &name) const
 	{
 		return stoi(get(name), -32768, 32767);
 	}
 
-	s32 getS32(std::string name) const
+	s32 getS32(const std::string &name) const
 	{
 		return stoi(get(name));
 	}
 
-	v3f getV3F(std::string name) const
+	v3f getV3F(const std::string &name) const
 	{
 		v3f value;
 		Strfnd f(get(name));
@@ -509,7 +510,7 @@ public:
 		return value;
 	}
 
-	v2f getV2F(std::string name) const
+	v2f getV2F(const std::string &name) const
 	{
 		v2f value;
 		Strfnd f(get(name));
@@ -519,7 +520,7 @@ public:
 		return value;
 	}
 
-	u64 getU64(std::string name) const
+	u64 getU64(const std::string &name) const
 	{
 		u64 value = 0;
 		std::string s = get(name);
@@ -528,7 +529,8 @@ public:
 		return value;
 	}
 
-	u32 getFlagStr(std::string name, FlagDesc *flagdesc, u32 *flagmask) const
+	u32 getFlagStr(const std::string &name, const FlagDesc *flagdesc,
+			u32 *flagmask) const
 	{
 		std::string val = get(name);
 		return std::isdigit(val[0])
@@ -538,8 +540,8 @@ public:
 
 	// N.B. if getStruct() is used to read a non-POD aggregate type,
 	// the behavior is undefined.
-	bool getStruct(std::string name, std::string format,
-				   void *out, size_t olen) const
+	bool getStruct(const std::string &name, const std::string &format,
+			void *out, size_t olen) const
 	{
 		std::string valstr;
 
@@ -556,7 +558,7 @@ public:
 	}
 
 	//////////// Try to get value, no exception thrown
-	bool getNoEx(std::string name, std::string &val) const
+	bool getNoEx(const std::string &name, std::string &val) const
 	{
 		try {
 			val = get(name);
@@ -569,7 +571,7 @@ public:
 	// N.B. getFlagStrNoEx() does not set val, but merely modifies it.  Thus,
 	// val must be initialized before using getFlagStrNoEx().  The intention of
 	// this is to simplify modifying a flags field from a default value.
-	bool getFlagStrNoEx(std::string name, u32 &val, FlagDesc *flagdesc) const
+	bool getFlagStrNoEx(const std::string &name, u32 &val, FlagDesc *flagdesc) const
 	{
 		try {
 			u32 flags, flagmask;
@@ -585,7 +587,7 @@ public:
 		}
 	}
 
-	bool getFloatNoEx(std::string name, float &val) const
+	bool getFloatNoEx(const std::string &name, float &val) const
 	{
 		try {
 			val = getFloat(name);
@@ -595,7 +597,7 @@ public:
 		}
 	}
 
-	bool getU16NoEx(std::string name, int &val) const
+	bool getU16NoEx(const std::string &name, int &val) const
 	{
 		try {
 			val = getU16(name);
@@ -605,7 +607,7 @@ public:
 		}
 	}
 
-	bool getU16NoEx(std::string name, u16 &val) const
+	bool getU16NoEx(const std::string &name, u16 &val) const
 	{
 		try {
 			val = getU16(name);
@@ -615,7 +617,7 @@ public:
 		}
 	}
 
-	bool getS16NoEx(std::string name, int &val) const
+	bool getS16NoEx(const std::string &name, int &val) const
 	{
 		try {
 			val = getU16(name);
@@ -625,7 +627,7 @@ public:
 		}
 	}
 
-	bool getS16NoEx(std::string name, s16 &val) const
+	bool getS16NoEx(const std::string &name, s16 &val) const
 	{
 		try {
 			val = getS16(name);
@@ -635,7 +637,7 @@ public:
 		}
 	}
 
-	bool getS32NoEx(std::string name, s32 &val) const
+	bool getS32NoEx(const std::string &name, s32 &val) const
 	{
 		try {
 			val = getS32(name);
@@ -645,7 +647,7 @@ public:
 		}
 	}
 
-	bool getV3FNoEx(std::string name, v3f &val) const
+	bool getV3FNoEx(const std::string &name, v3f &val) const
 	{
 		try {
 			val = getV3F(name);
@@ -655,7 +657,7 @@ public:
 		}
 	}
 
-	bool getV2FNoEx(std::string name, v2f &val) const
+	bool getV2FNoEx(const std::string &name, v2f &val) const
 	{
 		try {
 			val = getV2F(name);
@@ -665,7 +667,7 @@ public:
 		}
 	}
 
-	bool getU64NoEx(std::string name, u64 &val) const
+	bool getU64NoEx(const std::string &name, u64 &val) const
 	{
 		try {
 			val = getU64(name);
@@ -679,7 +681,7 @@ public:
 
 	// N.B. if setStruct() is used to write a non-POD aggregate type,
 	// the behavior is undefined.
-	bool setStruct(std::string name, std::string format, void *value)
+	bool setStruct(const std::string &name, const std::string &format, void *value)
 	{
 		std::string structstr;
 		if (!serializeStructToString(&structstr, format, value))
@@ -689,50 +691,47 @@ public:
 		return true;
 	}
 
-	void setFlagStr(std::string name, u32 flags,
-		FlagDesc *flagdesc, u32 flagmask)
+	void setFlagStr(const std::string &name, u32 flags,
+		const FlagDesc *flagdesc, u32 flagmask)
 	{
 		set(name, writeFlagString(flags, flagdesc, flagmask));
 	}
 
-	void setBool(std::string name, bool value)
+	void setBool(const std::string &name, bool value)
 	{
-		if(value)
-			set(name, "true");
-		else
-			set(name, "false");
+		set(name, value ? "true" : "false");
 	}
 
-	void setFloat(std::string name, float value)
+	void setFloat(const std::string &name, float value)
 	{
 		set(name, ftos(value));
 	}
 
-	void setV3F(std::string name, v3f value)
+	void setV3F(const std::string &name, v3f value)
 	{
 		std::ostringstream os;
 		os<<"("<<value.X<<","<<value.Y<<","<<value.Z<<")";
 		set(name, os.str());
 	}
 
-	void setV2F(std::string name, v2f value)
+	void setV2F(const std::string &name, v2f value)
 	{
 		std::ostringstream os;
 		os<<"("<<value.X<<","<<value.Y<<")";
 		set(name, os.str());
 	}
 
-	void setS16(std::string name, s16 value)
+	void setS16(const std::string &name, s16 value)
 	{
 		set(name, itos(value));
 	}
 
-	void setS32(std::string name, s32 value)
+	void setS32(const std::string &name, s32 value)
 	{
 		set(name, itos(value));
 	}
 
-	void setU64(std::string name, u64 value)
+	void setU64(const std::string &name, u64 value)
 	{
 		std::ostringstream os;
 		os<<value;
@@ -747,60 +746,54 @@ public:
 		m_defaults.clear();
 	}
 
-	void updateValue(Settings &other, const std::string &name)
+	void updateValue(const Settings &other, const std::string &name)
 	{
-		JMutexAutoLock lock(m_mutex);
-
-		if(&other == this)
+		if (&other == this)
 			return;
 
-		try{
+		JMutexAutoLock lock(m_mutex);
+
+		try {
 			std::string val = other.get(name);
 			m_settings[name] = val;
-		} catch(SettingNotFoundException &e){
+		} catch (SettingNotFoundException &e) {
 		}
 
 		return;
 	}
 
-	void update(Settings &other)
+	void update(const Settings &other)
 	{
+		if (&other == this)
+			return;
+
 		JMutexAutoLock lock(m_mutex);
 		JMutexAutoLock lock2(other.m_mutex);
 
-		if(&other == this)
-			return;
-
 		m_settings.insert(other.m_settings.begin(), other.m_settings.end());
 		m_defaults.insert(other.m_defaults.begin(), other.m_defaults.end());
 
 		return;
 	}
 
-	Settings & operator+=(Settings &other)
+	Settings & operator+=(const Settings &other)
 	{
-		JMutexAutoLock lock(m_mutex);
-		JMutexAutoLock lock2(other.m_mutex);
-
-		if(&other == this)
-			return *this;
-
 		update(other);
-
 		return *this;
 
 	}
 
-	Settings & operator=(Settings &other)
+	Settings & operator=(const Settings &other)
 	{
+		if (&other == this)
+			return *this;
+
 		JMutexAutoLock lock(m_mutex);
 		JMutexAutoLock lock2(other.m_mutex);
 
-		if(&other == this)
-			return *this;
 
 		clear();
-		(*this) += other;
+		update(other);
 
 		return *this;
 	}
diff --git a/src/util/string.cpp b/src/util/string.cpp
index 363a15e65..92a4735c4 100644
--- a/src/util/string.cpp
+++ b/src/util/string.cpp
@@ -191,7 +191,7 @@ std::string urldecode(std::string str)
 	return oss.str();
 }
 
-u32 readFlagString(std::string str, FlagDesc *flagdesc, u32 *flagmask)
+u32 readFlagString(std::string str, const FlagDesc *flagdesc, u32 *flagmask)
 {
 	u32 result = 0, mask = 0;
 	char *s = &str[0];
@@ -225,7 +225,7 @@ u32 readFlagString(std::string str, FlagDesc *flagdesc, u32 *flagmask)
 	return result;
 }
 
-std::string writeFlagString(u32 flags, FlagDesc *flagdesc, u32 flagmask)
+std::string writeFlagString(u32 flags, const FlagDesc *flagdesc, u32 flagmask)
 {
 	std::string result;
 
diff --git a/src/util/string.h b/src/util/string.h
index 6e2db0af4..98d23b161 100644
--- a/src/util/string.h
+++ b/src/util/string.h
@@ -334,8 +334,8 @@ inline bool is_number(const std::string& tocheck)
 std::string translatePassword(std::string playername, std::wstring password);
 std::string urlencode(std::string str);
 std::string urldecode(std::string str);
-u32 readFlagString(std::string str, FlagDesc *flagdesc, u32 *flagmask);
-std::string writeFlagString(u32 flags, FlagDesc *flagdesc, u32 flagmask);
+u32 readFlagString(std::string str, const FlagDesc *flagdesc, u32 *flagmask);
+std::string writeFlagString(u32 flags, const FlagDesc *flagdesc, u32 flagmask);
 size_t mystrlcpy(char *dst, const char *src, size_t size);
 char *mystrtok_r(char *s, const char *sep, char **lasts);
 u64 read_seed(const char *str);