Add DEPRECATED macro and remove uses of snprintf

src/global.hpp

@@ -20,9 +20,6 @@
 // Enable C99 support for VC14
 #if _MSC_VER>=1900
 #define STDC99
-#else
-#undef snprintf
-#define snprintf _snprintf
 #endif
 
 // Disable warning about source encoding not in current code page.
@@ -31,9 +28,6 @@
 // Disable warning about deprecated functions.
 #pragma warning(disable: 4996)
 
-// Disable warning when using time_t in snprintf.
-#pragma warning(disable: 4477)
-
 // Disable some MSVC warnings which are useless according to mordante
 #pragma warning(disable: 4244)
 #pragma warning(disable: 4345)
@@ -46,7 +40,7 @@
 #ifdef NDEBUG
 /*
  * Wesnoth uses asserts to avoid undefined behaviour. For example, to make sure
- * pointers are not nullptr before deferring them, or collections are not empty
+ * pointers are not nullptr before dereferencing them, or collections are not empty
  * before accessing their elements. Therefore Wesnoth should not be compiled
  * with assertions disabled.
  */
@@ -75,13 +69,20 @@
 #define NOEXCEPT throw()
 #define NORETURN __declspec(noreturn)
 #endif
+// MSVC supports these starting in 2017?
+// Some sources claim MSVC 2015 supports them, but let's be safe...
+#if _MSC_VER >= 1910
+#define DEPRECATED(reason) [[deprecated(reason)]]
+#else
+#define DEPRECATED(reason) __declspec(deprecated)
+#endif
 #endif
 
 #if defined(__clang__)
 #include <ciso646> // To ensure standard library version macros are defined
 // If it's libc++, no problem. Otherwise, attempt to detect libstdc++ version (needs GCC 5.1 or higher)
 // by testing for the existence of a header added in that version.
-#if defined(_LIBCPP_VERSION) || __has_include(<experimental/any>)
+#if defined(_LIBCPP_VERSION) || __has_include(<experimental/any>) || __has_include(<any>)
 #define HAVE_PUT_TIME 1
 #else
 #define HAVE_PUT_TIME 0
@@ -93,6 +94,12 @@
 // All supported versions of clang have this
 #define NORETURN [[noreturn]]
 
+#if __has_cpp_attribute(deprecated)
+#define DEPRECATED(reason) [[deprecated(reason)]]
+#else
+#define DEPRECATED(reason) __attribute__((deprecated(reason)))
+#endif
+
 #if __has_feature(cxx_constexpr)
 #define CONSTEXPR constexpr
 #else
@@ -115,6 +122,13 @@
 #define NORETURN [[noreturn]]
 #define HAVE_REF_QUALIFIERS 1
 #define HAVE_INHERITING_CTORS 1
+
+// Deprecated is supported from 4.9 up
+#if __GNUC__ >= 5 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 9)
+#define DEPRECATED(reason) [[deprecated(reason)]]
+#else
+#define DEPRECATED(reason) __attribute__((deprecated(reason)))
+#endif
 #endif
 
 #endif //GLOBAL_HPP_INCLUDED

src/reports.cpp

@@ -14,8 +14,6 @@
 
 #include "actions/attack.hpp"
 #include "attack_prediction.hpp"
-//#include "editor/editor_controller.hpp"
-//#include "editor/palette/terrain_palettes.hpp"
 #include "font/pango/escape.hpp"
 #include "font/text_formatting.hpp"
 #include "formatter.hpp"
@@ -826,31 +824,21 @@ static int attack_info(reports::context & rc, const attack_type &at, config &res
 }
 
 // Conversion routine for both unscathed and damage change percentage.
-static void format_prob(char str_buf[10], double prob)
+static std::string format_prob(double prob)
 {
-
 	if(prob > 0.9995) {
-		snprintf(str_buf, 10, "100 %%");
+		return "100%";
-	} else if(prob >= 0.1) {
-		snprintf(str_buf, 10, "%4.1f %%", 100.0 * prob);
-	} else {
-		snprintf(str_buf, 10, " %3.1f %%", 100.0 * prob);
 	}
-
+	std::ostringstream res;
-	str_buf[9] = '\0';  //prevents _snprintf error
+	res << std::setprecision(1) << std::setw(4) << 100.0 * prob << "%";
+	return res.str();
 }
 
-static void format_hp(char str_buf[10], int hp)
+static std::string format_hp(unsigned hp)
 {
-	if(hp < 10) {
+	std::ostringstream res;
-		snprintf(str_buf, 10, "   %i", hp);
+	res << ' ' << std::setw(3) << hp;
-	} else if(hp < 99) {
+	return res.str();
-		snprintf(str_buf, 10, "  %i", hp);
-	} else {
-		snprintf(str_buf, 10, " %i", hp);
-	}
-
-	str_buf[9] = '\0';  //prevents _snprintf error
 }
 
 static config unit_weapons(reports::context & rc, const unit *attacker, const map_location &attacker_pos, const unit *defender, bool show_attacker)
@@ -954,23 +942,16 @@ static config unit_weapons(reports::context & rc, const unit *attacker, const ma
 		// And reverse the order. Might be doable in a better manor.
 		std::reverse(hp_prob_vector.begin(), hp_prob_vector.end());
 
-		for(i = 0;
+		for(i = 0; i < static_cast<int>(hp_prob_vector.size()); i++) {
-				i < static_cast<int>(hp_prob_vector.size()); i++) {
 
 			int hp = hp_prob_vector[i].first;
 			double prob = hp_prob_vector[i].second;
-
-			char prob_buf[10];
-			format_prob(prob_buf, prob);
-			char hp_buf[10];
-			format_hp(hp_buf, hp);
-
 			color_t prob_color = game_config::blue_to_white(prob * 100.0, true);
 
 			str		<< span_color(font::weapon_details_color) << "  " << "  "
-					<< span_color(u->hp_color(hp)) << hp_buf << naps
+					<< span_color(u->hp_color(hp)) << format_hp(hp) << naps
 					<< " " << font::weapon_numbers_sep << " "
-					<< span_color(prob_color) << prob_buf << naps
+					<< span_color(prob_color) << format_prob(prob) << naps
 					<< naps << "\n";
 		}
 

src/serialization/string_utils.cpp

@@ -429,30 +429,31 @@ std::string unescape(const std::string &str)
 
 std::string urlencode(const std::string &str)
 {
-	static std::string nonresv =
+	static const std::string nonresv_str =
 		"-."
 		"0123456789"
 		"ABCDEFGHIJKLMNOPQRSTUVWXYZ"
 		"_"
 		"abcdefghijklmnopqrstuvwxyz"
 		"~";
+	static const std::set<char> nonresv(nonresv_str.begin(), nonresv_str.end());
 
-	std::string res;
+	std::ostringstream res;
+	res << std::hex;
+	res.fill('0');
 
-	for(size_t n = 0; n < str.length(); ++n) {
+	for(char c : str) {
-		const char& c = str[n];
+		if(nonresv.count(c) == 0) {
-
+			res << c;
-		if(nonresv.find(c) != std::string::npos) {
-			res += c;
 			continue;
 		}
 
-		char buf[4];
+		res << '%';
-		snprintf(buf, sizeof(buf), "%%%02X", c);
+		res.width(2);
-		res += buf;
+		res << int(c);
 	}
 
-	return res;
+	return res.str();
 }
 
 bool string_bool(const std::string& str, bool def) {

src/server/game.cpp

@@ -170,14 +170,15 @@ bool game::is_player(const socket_ptr player) const {
 namespace {
 std::string describe_turns(int turn, int num_turns)
 {
-	char buf[100];
+	std::ostringstream buf;
+	buf << turn << '/';
 
 	if(num_turns == -1) {
-		snprintf(buf, sizeof(buf), "%d/-", turn);
+		buf << '-';
 	} else {
-		snprintf(buf, sizeof(buf), "%d/%d", turn, num_turns);
+		buf << num_turns;
 	}
-	return buf;
+	return buf.str();
 }
 
 }//anon namespace
@@ -614,11 +615,12 @@ bool game::describe_slots() {
 			++available_slots;
 		}
 	}
-	char buf[50];
+	std::ostringstream buf;
-	snprintf(buf,sizeof(buf), "%d/%d", available_slots, num_sides);
+	buf << available_slots << '/' << num_sides;
+	std::string descr = buf.str();
 
-	if ((*description_)["slots"] != buf) {
+	if ((*description_)["slots"] != descr) {
-		description_->set_attr_dup("slots", buf);
+		description_->set_attr_dup("slots", descr);
 		return true;
 	} else {
 		return false;

src/units/unit.hpp

@@ -278,9 +278,7 @@ public:
 		id_ = id;
 	}
 
-#ifdef HAVE_CXX14
+	DEPRECATED("Use comparison against id() instead")
-	[[deprecated("Use comparison against id() instead")]]
-#endif
 	bool matches_id(const std::string& unit_id) const
 	{
 		return id_ == unit_id;