# small fixes.

add translate_free (avoids mem leak in translate API)
debug: when skipping errors but an unexpected one is raised, trigger a
warning.
test_archive_builder: properly clean up archive; decrease number of
files to speed things up.

This was SVN commit r4035.

source/lib/app_hooks.cpp

@@ -109,13 +109,18 @@ static void bundle_logs(FILE* f)
 }
 
 
-
 static const wchar_t* translate(const wchar_t* text)
 {
 	return text;
 }
 
 
+static void translate_free(const wchar_t* text)
+{
+	// no-op - translate() doesn't own the pointer.
+}
+
+
 static void log(const wchar_t* text)
 {
 	wprintf(text);
@@ -131,7 +136,7 @@ static ErrorReaction display_error(const wchar_t* UNUSED(text), uint UNUSED(flag
 //-----------------------------------------------------------------------------
 
 // contains the current set of hooks. starts with the stub values and
-// may be changed via set_app_hooks.
+// may be changed via app_hooks_update.
 //
 // rationale: we don't ever need to switch "hook sets", so one global struct
 // is fine. by always having one defined, we also avoid having to check
@@ -149,7 +154,7 @@ static AppHooks ah =
 // register the specified hook function pointers. any of them that
 // are non-zero override the previous function pointer value
 // (these default to the stub hooks which are functional but basic).
-void set_app_hooks(AppHooks* ah_)
+void app_hooks_update(AppHooks* ah_)
 {
 	debug_assert(ah_);
 	ONCE_NOT(debug_warn("app hooks already set"));

source/lib/debug.cpp

@@ -656,13 +656,16 @@ ErrorReaction debug_display_error(const wchar_t* description,
 
 
 
-static uintptr_t will_skip_next_error;	// set/reset via CAS for thread-safety.
+// strobe indicating expected_err is valid and the next error should be
-static LibError err_to_skip;
+// compared against that / skipped if equal to it.
+// set/reset via CAS for thread-safety (hence uintptr_t).
+static uintptr_t expected_err_valid;
+static LibError expected_err;
 
 void debug_skip_next_err(LibError err)
 {
-	if(CAS(&will_skip_next_error, 0, 1))
+	if(CAS(&expected_err_valid, 0, 1))
-		err_to_skip = err;
+		expected_err = err;
 	else
 		debug_warn("internal error: concurrent attempt to skip assert/error");
 
@@ -670,11 +673,15 @@ void debug_skip_next_err(LibError err)
 
 static bool should_skip_this_error(LibError err)
 {
-	// (compare before resetting strobe - err_to_skip may change afterwards)
+	// (compare before resetting strobe - expected_err may change afterwards)
-	bool should_skip = (err_to_skip == err);
+	bool was_expected_err = (expected_err == err);
 	// (use CAS to ensure only one error is skipped)
-	if(CAS(&will_skip_next_error, 1, 0))
+	if(CAS(&expected_err_valid, 1, 0))
-		return should_skip;
+	{
+		if(!was_expected_err)
+			debug_warn("anticipated error was not raised");
+		return was_expected_err;
+	}
 
 	return false;
 }

source/lib/debug.h

@@ -397,8 +397,8 @@ extern ErrorReaction debug_warn_err(LibError err,
  * we therefore need to squelch them.
  *
  * @param err the LibError to skip. if the next error to be raised matches
- * this, it is skipped. after that (regardless of whether it matched),
+ * this, it is skipped. otherwise, we raise a warning to help catch
- * the skip request is reset, i.e. forgotten.
+ * erroneous usage. either way, the skip request is reset afterwards.
  *
  * note: this is thread-safe, but to prevent confusion, only one
  * concurrent skip request is allowed.

source/lib/res/file/tests/test_archive_builder.h

@@ -10,7 +10,7 @@
 class TestArchiveBuilder : public CxxTest::TestSuite 
 {
 	const char* const archive_fn;
-	static const size_t NUM_FILES = 100;
+	static const size_t NUM_FILES = 30;
 	static const size_t MAX_FILE_SIZE = 20000;
 
 	std::set<const char*> existing_names;
@@ -96,6 +96,7 @@ public:
 	{
 		vfs_shutdown();
 		dir_delete("archivetest");
+		file_delete(archive_fn);
 	}
 
 	void test_create_archive_with_random_files()

source/ps/GameSetup/GameSetup.cpp

@@ -919,7 +919,8 @@ void Init(int argc, char* argv[], uint flags)
 	// override ah_translate with our i18n code.
 	AppHooks hooks = {0};
 	hooks.translate = psTranslate;
-	set_app_hooks(&hooks);
+	hooks.translate_free = psTranslateFree;
+	app_hooks_update(&hooks);
 
 	// Set up the console early, so that debugging
 	// messages can be logged to it. (The console's size

source/ps/Pyrogenesis.cpp

@@ -6,11 +6,12 @@
 DEFINE_ERROR(PS_OK, "OK");
 DEFINE_ERROR(PS_FAIL, "Fail");
 
+
+static const wchar_t* translate_no_mem = L"(no mem)";
+
 // overrides ah_translate. registered in GameSetup.cpp
 const wchar_t* psTranslate(const wchar_t* text)
 {
-	// TODO: leaks memory returned by wcsdup
-
 	// make sure i18n system is (already|still) initialized.
 	if(g_CurrentLocale)
 	{
@@ -19,15 +20,21 @@ const wchar_t* psTranslate(const wchar_t* text)
 		try
 		{
 			CStrW ret = I18n::translate(text);
-			const wchar_t* text2 = wcsdup(ret.c_str());
+			const wchar_t* ret_dup = wcsdup(ret.c_str());
-			// only overwrite if wcsdup succeeded, i.e. not out of memory.
+			return ret_dup? ret_dup : translate_no_mem;
-			if(text2)
-				text = text2;
 		}
 		catch(...)
 		{
 		}
 	}
 
-	return text;
+	// i18n not available: at least try and return the text (unchanged)
+	const wchar_t* ret_dup = wcsdup(text);
+	return ret_dup? ret_dup : translate_no_mem;
+}
+
+void psTranslateFree(const wchar_t* text)
+{
+	if(text != translate_no_mem)
+		free((void*)text);
 }

source/ps/Pyrogenesis.h

@@ -25,5 +25,6 @@ DECLARE_ERROR(PS_FAIL);
 
 // overrides ah_translate. registered in GameSetup.cpp
 extern const wchar_t* psTranslate(const wchar_t* text);
+extern void psTranslateFree(const wchar_t* text);
 
 #endif