widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

SirVer has proposed merging lp:~widelands-dev/widelands/sdl2_logging into lp:widelands.

Commit message:
- Removes --logfile command line flag. Use redirects instead.
- Adds a SDL2 aware logger that replicates writing a stdout.txt on windows (untested).

I think this a reasonable first-snow, minimal invasive fix to bring back a similar logging behavior that we had with SDL 1.2. I am unsure if always writing a logfile is such a great idea performance wise, but it never came up as a bottleneck, so it will probably be fine. 

Requested reviews:
  Tino (tino79)
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1386741 in widelands: "Fix logging"
  https://bugs.launchpad.net/widelands/+bug/1386741

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/sdl2_logging/+merge/302866

Tino, could you test this branch (and fix the compile errors I most certainly made?) - I still do not have access to a windows machine for building. 
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/sdl2_logging into lp:widelands.

=== modified file 'src/base/log.cc'
--- src/base/log.cc	2016-08-04 15:49:05 +0000
+++ src/base/log.cc	2016-08-13 12:29:25 +0000
@@ -18,24 +18,110 @@
  */
 
 #include "base/log.h"
+#include "base/macros.h"
+
+#include <SDL2/SDL.h>
 
 #include <cstdarg>
 #include <cstdio>
+#include <fstream>
 #include <iostream>
+#ifdef _WIN32
+#include <windows.h>
+#endif
+
+namespace {
+
+// Forward declaration to work around cyclic dependency.
+void sdl_logging_func(void* userdata, int, SDL_LogPriority, const char* message);
+
+#ifdef _WIN32
+
+string get_output_directory() {
+	// This took inspiration from SDL 1.2 logger code.
+#ifdef _WIN32_WCE
+	wchar_t path[MAX_PATH];
+#else
+	char path[MAX_PATH];
+#endif
+	const auto pathlen = GetModuleFileName(NULL, path, MAX_PATH);
+	while (pathlen > 0 && path[pathlen] != '\\') {
+		--pathlen;
+	}
+	path[pathlen] = '\0';
+	return path;
+}
+
+// This Logger emulates the SDL1.2 behavior of writing a stdout.txt.
+class WindowsLogger {
+public:
+	WindowsLogger() : stdout_filename_(get_output_directory() + "\\stdout.txt") {
+		stdout_.open(stdout_filename_);
+		if (!stdout_.good()) {
+			throw wexception("Unable to initialize logging to stdout.txt");
+		}
+		SDL_LogSetOutputFunction(sdl_logging_func, this);
+	}
+
+	void log_cstring(const char* buffer) {
+		stdout_ << buffer;
+		stdout_.flush();
+	}
+
+private:
+	const string stdout_filename_;
+	ofstream stdout_;
+
+	DISALLOW_COPY_AND_ASSIGN(WindowsLogger);
+};
+
+void sdl_logging_func(void* userdata,
+                      int /* category */,
+                      SDL_LogPriority /* priority */,
+                      const char* message) {
+	static_cast<WindowsLogger*>(userdata)->log_cstring(message);
+}
+#else // _WIN32
+
+class Logger {
+public:
+	Logger() {
+		SDL_LogSetOutputFunction(sdl_logging_func, this);
+	}
+
+	void log_cstring(const char* buffer) {
+		std::cout << buffer;
+		std::cout.flush();
+	}
+
+private:
+	DISALLOW_COPY_AND_ASSIGN(Logger);
+};
+
+void sdl_logging_func(void* userdata,
+                      int /* category */,
+                      SDL_LogPriority /* priority */,
+                      const char* message) {
+	static_cast<Logger*>(userdata)->log_cstring(message);
+}
+#endif
+
+}  // namespace
 
 // Default to stdout for logging.
-std::ostream& wout = std::cout;
-
 bool g_verbose = false;
 
 void log(const char* const fmt, ...) {
+#ifdef _WIN32
+	static WindowsLogger logger;
+#else 
+	static Logger logger;
+#endif 
 	char buffer[2048];
 	va_list va;
 
 	va_start(va, fmt);
 	vsnprintf(buffer, sizeof(buffer), fmt, va);
 	va_end(va);
-
-	wout << buffer;
-	wout.flush();
+	logger.log_cstring(buffer);
 }

=== modified file 'src/base/log.h'
--- src/base/log.h	2016-08-04 15:49:05 +0000
+++ src/base/log.h	2016-08-13 12:29:25 +0000
@@ -46,9 +46,7 @@
 #define PRIXS PRIS_PREFIX "X"
 #define PRIoS PRIS_PREFIX "o"
 
-// Print a formatted log messages to wout.
-// wout is either std::cout or specified logfile.
-extern std::ostream& wout;
+// Print a formatted log messages to stdout on most systems and 'stdout.txt' on windows.
 void log(const char*, ...) PRINTF_FORMAT(1, 2);
 
 extern bool g_verbose;

=== modified file 'src/wlapplication.cc'
--- src/wlapplication.cc	2016-08-04 15:49:05 +0000
+++ src/wlapplication.cc	2016-08-13 12:29:25 +0000
@@ -832,18 +832,6 @@
  * true otherwise.
 */
 void WLApplication::handle_commandline_parameters() {
-	if (commandline_.count("logfile")) {
-		logfile_ = commandline_["logfile"];
-		std::cerr << "Redirecting log target to: " << logfile_ << std::endl;
-		if (logfile_.size() != 0) {
-			// TODO(unknown): (very small) memory leak of 1 ofstream;
-			// swaw the buffers (internally) of the file and wout
-			std::ofstream* widelands_out = new std::ofstream(logfile_.c_str());
-			std::streambuf* logbuf = widelands_out->rdbuf();
-			wout.rdbuf(logbuf);
-		}
-		commandline_.erase("logfile");
-	}
 	if (commandline_.count("nosound")) {
 		g_sound_handler.nosound_ = true;
 		commandline_.erase("nosound");

=== modified file 'src/wlapplication.h'
--- src/wlapplication.h	2016-08-04 15:49:05 +0000
+++ src/wlapplication.h	2016-08-13 12:29:25 +0000
@@ -229,9 +229,6 @@
 	/// --scenario or --loadgame.
 	std::string script_to_run_;
 
-	// Log all output to this file if set, otherwise use cout
-	std::string logfile_;
-
 	GameType game_type_;
 
 	/// True if left and right mouse button should be swapped

=== modified file 'src/wlapplication_messages.cc'
--- src/wlapplication_messages.cc	2016-08-04 15:49:05 +0000
+++ src/wlapplication_messages.cc	2016-08-13 12:29:25 +0000
@@ -44,9 +44,6 @@
 	std::cout << _("Options:") << endl << endl;
 	std::cout << _(" --<config-entry-name>=value overwrites any config file setting") << endl
 	          << endl
-	          << _(" --logfile=FILENAME   Log output to file FILENAME instead of \n"
-	               "                      terminal output")
-	          << endl
 	          << _(" --datadir=DIRNAME    Use specified directory for the widelands\n"
 	               "                      data files")
 	          << endl