Remove use of non-threadsafe strerror

Add helper class to use strerror_r or strerror_s

src/music/extmidi.cpp

@@ -125,7 +125,7 @@ void MusicDriver_ExtMidi::DoPlay()
 		}
 
 		case -1:
-			DEBUG(driver, 0, "extmidi: couldn't fork: %s", strerror(errno));
+			DEBUG(driver, 0, "extmidi: couldn't fork: %s", StrErrorDumper().GetLast());
 			[[fallthrough]];
 
 		default:

src/network/core/os_abstraction.cpp

@@ -90,14 +90,7 @@ const char *NetworkError::AsString() const
 			this->message.assign(FS2OTTD(buffer));
 		}
 #else
-		/* Make strerror thread safe by locking access to it. There is a thread safe strerror_r, however
-		 * the non-POSIX variant is available due to defining _GNU_SOURCE meaning it is not portable.
-		 * The problem with the non-POSIX variant is that it does not necessarily fill the buffer with
-		 * the error message but can also return a pointer to a static bit of memory, whereas the POSIX
-		 * variant always fills the buffer. This makes the behaviour too erratic to work with. */
-		static std::mutex mutex;
-		std::lock_guard<std::mutex> guard(mutex);
-		this->message.assign(strerror(this->error));
+		this->message.assign(StrErrorDumper().Get(this->error));
 #endif
 	}
 	return this->message.c_str();

src/os/unix/crashlog_unix.cpp

@@ -275,7 +275,7 @@ class CrashLogUnix : public CrashLog {
 	{
 		struct utsname name;
 		if (uname(&name) < 0) {
-			return buffer + seprintf(buffer, last, "Could not get OS version: %s\n", strerror(errno));
+			return buffer + seprintf(buffer, last, "Could not get OS version: %s\n", StrErrorDumper().GetLast());
 		}
 
 		return buffer + seprintf(buffer, last,

src/string.cpp

@@ -1392,3 +1392,32 @@ class DefaultStringIterator : public StringIterator
 #endif /* defined(WITH_COCOA) && !defined(STRGEN) && !defined(SETTINGSGEN) */
 
 #endif
+
+const char *StrErrorDumper::Get(int errornum)
+{
+#if defined(_WIN32)
+	if (strerror_s(this->buf, lengthof(this->buf), errornum) == 0) {
+		return this->buf;
+	}
+#else
+	auto result = strerror_r(errornum, this->buf, lengthof(this->buf));
+	static_assert(std::is_same_v<decltype(result), char *> || std::is_same_v<decltype(result), int>);
+	if constexpr (std::is_same_v<decltype(result), char *>) {
+		/* GNU-specific */
+		return result;
+	} else {
+		/* XSI-compliant */
+		if (result == 0) {
+			return this->buf;
+		}
+	}
+#endif
+
+	seprintf(this->buf, lastof(this->buf), "Unknown error %d", errornum);
+	return this->buf;
+}
+
+const char *StrErrorDumper::GetLast()
+{
+	return this->Get(errno);
+}

src/string_func.h

@@ -306,4 +306,17 @@ inline bool IsWhitespace(char32_t c)
 char *strcasestr(const char *haystack, const char *needle);
 #endif /* strcasestr is available */
 
+/**
+ * The use of a struct is so that when used as an argument to seprintf/etc, the buffer lives
+ * on the stack with a lifetime which lasts until the end of the statement.
+ * This avoids using a static buffer which is thread-unsafe, or needing to call malloc, which would then nee to be freed.
+ */
+struct StrErrorDumper {
+	const char *Get(int errornum);
+	const char *GetLast();
+
+private:
+	char buf[128];
+};
+
 #endif /* STRING_FUNC_H */