From b8ee5b3c7698db0fb5e71875fc1901b1281b0421 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Tue, 23 Oct 2018 22:31:42 +0200
Subject: [PATCH 01/19] begin the work to swap strings to database

---
 src/eressea.c             |   5 +-
 src/kernel/CMakeLists.txt |   1 +
 src/kernel/db/driver.h    |   2 +
 src/kernel/db/sqlite.c    | 151 +++++++++++++++++++++++++++-----------
 src/kernel/order.c        |  18 +++++
 src/orderdb.c             |  28 -------
 6 files changed, 133 insertions(+), 72 deletions(-)

diff --git a/src/eressea.c b/src/eressea.c
index e1767811c..1221f93e0 100644
--- a/src/eressea.c
+++ b/src/eressea.c
@@ -3,6 +3,7 @@
 
 #include "kernel/building.h"
 #include "kernel/calendar.h"
+#include "kernel/database.h"
 #include "kernel/config.h"
 #include "kernel/curse.h"
 #include "kernel/equipment.h"
@@ -54,12 +55,12 @@ void game_done(void)
     free_locales();
 #endif
     kernel_done();
-    orderdb_close();
+    swapdb_close();
 }
 
 void game_init(void)
 {
-    orderdb_open();
+    swapdb_open();
     errno = 0;
     kernel_init();
     register_triggers();
diff --git a/src/kernel/CMakeLists.txt b/src/kernel/CMakeLists.txt
index 06f58fc1b..dcd19c35a 100644
--- a/src/kernel/CMakeLists.txt
+++ b/src/kernel/CMakeLists.txt
@@ -58,6 +58,7 @@ command.c
 config.c
 connection.c
 curse.c
+database.c
 equipment.c
 event.c
 faction.c
diff --git a/src/kernel/db/driver.h b/src/kernel/db/driver.h
index 9632cb5db..e127c6662 100644
--- a/src/kernel/db/driver.h
+++ b/src/kernel/db/driver.h
@@ -16,3 +16,5 @@ void db_driver_close(database_t db);
 int db_driver_order_save(const char *str);
 struct order_data *db_driver_order_load(int id);
 int db_driver_faction_save(int id, int no, int turn, const char *email, const char *password);
+unsigned int db_driver_string_save(const char *s);
+const char *db_driver_string_load(unsigned int id, size_t *size);
diff --git a/src/kernel/db/sqlite.c b/src/kernel/db/sqlite.c
index fd9d0bc67..9df27f365 100644
--- a/src/kernel/db/sqlite.c
+++ b/src/kernel/db/sqlite.c
@@ -15,14 +15,25 @@
 #include <string.h>
 
 static sqlite3 *g_game_db;
-static sqlite3 *g_temp_db;
+static sqlite3 *g_swap_db;
+static sqlite3_stmt * g_stmt_insert_string;
+static sqlite3_stmt * g_stmt_select_string;
 static sqlite3_stmt * g_stmt_insert_order;
 static sqlite3_stmt * g_stmt_select_order;
 static sqlite3_stmt * g_stmt_update_faction;
 static sqlite3_stmt * g_stmt_insert_faction;
 
-static int g_order_batchsize;
-static int g_order_tx_size;
+static int g_insert_batchsize;
+static int g_insert_tx_size;
+
+static void end_transaction(void) {
+    if (g_insert_tx_size > 0) {
+        int err;
+        g_insert_tx_size = 0;
+        err = sqlite3_exec(g_swap_db, "COMMIT", NULL, NULL, NULL);
+        assert(err == SQLITE_OK);
+    }
+}
 
 struct order_data *db_driver_order_load(int id)
 {
@@ -30,35 +41,27 @@ struct order_data *db_driver_order_load(int id)
     int err;
 
     ERRNO_CHECK();
-    if (g_order_tx_size > 0) {
-        g_order_tx_size = 0;
-        err = sqlite3_exec(g_temp_db, "COMMIT", NULL, NULL, NULL);
-        assert(err == SQLITE_OK);
-    }
+    end_transaction();
     err = sqlite3_reset(g_stmt_select_order);
     assert(err == SQLITE_OK);
     err = sqlite3_bind_int(g_stmt_select_order, 1, id);
     assert(err == SQLITE_OK);
-    do {
-        err = sqlite3_step(g_stmt_select_order);
-        if (err == SQLITE_ROW) {
-            const unsigned char *text;
-            int bytes;
-            bytes = sqlite3_column_bytes(g_stmt_select_order, 0);
-            assert(bytes > 0);
-            text = sqlite3_column_text(g_stmt_select_order, 0);
-            odata_create(&od, 1+(size_t)bytes, (const char *)text);
-            ERRNO_CHECK();
-            return od;
-        }
-    } while (err == SQLITE_ROW);
-    assert(err == SQLITE_DONE);
+    err = sqlite3_step(g_stmt_select_order);
+    if (err == SQLITE_ROW) {
+        const unsigned char *text;
+        int bytes;
+        text = sqlite3_column_text(g_stmt_select_order, 0);
+        bytes = sqlite3_column_bytes(g_stmt_select_order, 0);
+        assert(bytes > 0);
+        odata_create(&od, 1+(size_t)bytes, (const char *)text);
+        ERRNO_CHECK();
+        return od;
+    }
     ERRNO_CHECK();
     return NULL;
 }
 
-int db_driver_order_save(const char *str)
-{
+int db_driver_order_save(const char *str) {
     int err;
     sqlite3_int64 id;
     
@@ -66,9 +69,9 @@ int db_driver_order_save(const char *str)
    
     ERRNO_CHECK();
 
-    if (g_order_batchsize > 0) {
-        if (g_order_tx_size == 0) {
-            err = sqlite3_exec(g_temp_db, "BEGIN TRANSACTION", NULL, NULL, NULL);
+    if (g_insert_batchsize > 0) {
+        if (g_insert_tx_size == 0) {
+            err = sqlite3_exec(g_swap_db, "BEGIN TRANSACTION", NULL, NULL, NULL);
             assert(err == SQLITE_OK);
         }
     }
@@ -79,14 +82,12 @@ int db_driver_order_save(const char *str)
     assert(err == SQLITE_OK);
     err = sqlite3_step(g_stmt_insert_order);
     assert(err == SQLITE_DONE);
-    id = sqlite3_last_insert_rowid(g_temp_db);
+    id = sqlite3_last_insert_rowid(g_swap_db);
     assert(id <= INT_MAX);
     
-    if (g_order_batchsize > 0) {
-        if (++g_order_tx_size >= g_order_batchsize) {
-            err = sqlite3_exec(g_temp_db, "COMMIT", NULL, NULL, NULL);
-            assert(err == SQLITE_OK);
-            g_order_tx_size = 0;
+    if (g_insert_batchsize > 0) {
+        if (++g_insert_tx_size >= g_insert_batchsize) {
+            end_transaction();
         }
     }
     ERRNO_CHECK(); 
@@ -162,19 +163,25 @@ static int db_open_game(const char *dbname) {
 static int db_open_swap(const char *dbname) {
     int err;
 
-    g_order_batchsize = config_get_int("game.dbbatch", 100);
+    g_insert_batchsize = config_get_int("game.dbbatch", 100);
 
-    err = sqlite3_open(dbname, &g_temp_db);
+    err = sqlite3_open(dbname, &g_swap_db);
     assert(err == SQLITE_OK);
-    err = sqlite3_exec(g_temp_db, "PRAGMA journal_mode=OFF", NULL, NULL, NULL);
+    err = sqlite3_exec(g_swap_db, "PRAGMA journal_mode=OFF", NULL, NULL, NULL);
     assert(err == SQLITE_OK);
-    err = sqlite3_exec(g_temp_db, "PRAGMA synchronous=OFF", NULL, NULL, NULL);
+    err = sqlite3_exec(g_swap_db, "PRAGMA synchronous=OFF", NULL, NULL, NULL);
     assert(err == SQLITE_OK);
-    err = sqlite3_exec(g_temp_db, "CREATE TABLE IF NOT EXISTS orders (id INTEGER PRIMARY KEY, data TEXT NOT NULL)", NULL, NULL, NULL);
+    err = sqlite3_exec(g_swap_db, "CREATE TABLE IF NOT EXISTS orders (id INTEGER PRIMARY KEY, data TEXT NOT NULL)", NULL, NULL, NULL);
     assert(err == SQLITE_OK);
-    err = sqlite3_prepare_v2(g_temp_db, "INSERT INTO orders (data) VALUES (?)", -1, &g_stmt_insert_order, NULL);
+    err = sqlite3_exec(g_swap_db, "CREATE TABLE IF NOT EXISTS strings (id INTEGER PRIMARY KEY, data TEXT NOT NULL)", NULL, NULL, NULL);
     assert(err == SQLITE_OK);
-    err = sqlite3_prepare_v2(g_temp_db, "SELECT data FROM orders WHERE id=?", -1, &g_stmt_select_order, NULL);
+    err = sqlite3_prepare_v2(g_swap_db, "INSERT INTO strings (data) VALUES (?)", -1, &g_stmt_insert_string, NULL);
+    assert(err == SQLITE_OK);
+    err = sqlite3_prepare_v2(g_swap_db, "SELECT data FROM strings WHERE id=?", -1, &g_stmt_select_string, NULL);
+    assert(err == SQLITE_OK);
+    err = sqlite3_prepare_v2(g_swap_db, "INSERT INTO orders (data) VALUES (?)", -1, &g_stmt_insert_order, NULL);
+    assert(err == SQLITE_OK);
+    err = sqlite3_prepare_v2(g_swap_db, "SELECT data FROM orders WHERE id=?", -1, &g_stmt_select_order, NULL);
     assert(err == SQLITE_OK);
     ERRNO_CHECK();
     return 0;
@@ -202,12 +209,16 @@ void db_driver_close(database_t db)
 
     ERRNO_CHECK();
     if (db == DB_SWAP) {
-        assert(g_temp_db);
+        assert(g_swap_db);
+        err = sqlite3_finalize(g_stmt_select_string);
+        assert(err == SQLITE_OK);
+        err = sqlite3_finalize(g_stmt_insert_string);
+        assert(err == SQLITE_OK);
         err = sqlite3_finalize(g_stmt_select_order);
         assert(err == SQLITE_OK);
         err = sqlite3_finalize(g_stmt_insert_order);
         assert(err == SQLITE_OK);
-        err = sqlite3_close(g_temp_db);
+        err = sqlite3_close(g_swap_db);
         assert(err == SQLITE_OK);
         if (g_swapname) {
             FILE * F = fopen(g_swapname, "r");
@@ -229,3 +240,59 @@ void db_driver_close(database_t db)
     ERRNO_CHECK();
 }
 
+unsigned int db_driver_string_save(const char *str) {
+    int err;
+    sqlite3_int64 id;
+
+    assert(str);
+
+    ERRNO_CHECK();
+
+    if (g_insert_batchsize > 0) {
+        if (g_insert_tx_size == 0) {
+            err = sqlite3_exec(g_swap_db, "BEGIN TRANSACTION", NULL, NULL, NULL);
+            assert(err == SQLITE_OK);
+        }
+    }
+
+    err = sqlite3_reset(g_stmt_insert_string);
+    assert(err == SQLITE_OK);
+    err = sqlite3_bind_text(g_stmt_insert_string, 1, str, -1, SQLITE_STATIC);
+    assert(err == SQLITE_OK);
+    err = sqlite3_step(g_stmt_insert_string);
+    assert(err == SQLITE_DONE);
+    id = sqlite3_last_insert_rowid(g_swap_db);
+    assert(id <= INT_MAX);
+
+    if (g_insert_batchsize > 0) {
+        if (++g_insert_tx_size >= g_insert_batchsize) {
+            end_transaction();
+        }
+    }
+    ERRNO_CHECK();
+    return (int)id;
+}
+
+const char *db_driver_string_load(unsigned int id, size_t *size) {
+    int err;
+
+    end_transaction();
+    err = sqlite3_reset(g_stmt_select_string);
+    assert(err == SQLITE_OK);
+    err = sqlite3_bind_int(g_stmt_select_string, 1, id);
+    assert(err == SQLITE_OK);
+    err = sqlite3_step(g_stmt_select_string);
+    if (err == SQLITE_ROW) {
+        const unsigned char *text;
+        text = sqlite3_column_text(g_stmt_select_string, 0);
+        if (size) {
+            int bytes = sqlite3_column_bytes(g_stmt_select_string, 0);
+            assert(bytes > 0);
+            *size = (size_t)bytes;
+        }
+        ERRNO_CHECK();
+        return (const char *)text;
+    }
+    ERRNO_CHECK();
+    return NULL;
+}
diff --git a/src/kernel/order.c b/src/kernel/order.c
index c0c1e82fb..818c77a72 100644
--- a/src/kernel/order.c
+++ b/src/kernel/order.c
@@ -14,6 +14,8 @@
 #include <kernel/config.h>
 #include "order.h"
 
+#include "db/driver.h"
+
 #include "skill.h"
 
 #include <util/base36.h>
@@ -36,6 +38,22 @@
 # define ORD_KEYWORD(ord) (keyword_t)((ord)->command & 0xFFFF)
 # define OD_STRING(odata) ((odata) ? (odata)->_str : NULL)
 
+order_data *odata_load(int id)
+{
+    if (id > 0) {
+        return db_driver_order_load(id);
+    }
+    return NULL;
+}
+
+int odata_save(order_data *od)
+{
+    if (od->_str) {
+        return db_driver_order_save(od->_str);
+    }
+    return 0;
+}
+
 void odata_create(order_data **pdata, size_t len, const char *str)
 {
     order_data *data;
diff --git a/src/orderdb.c b/src/orderdb.c
index 5c224a79e..87b6e626c 100644
--- a/src/orderdb.c
+++ b/src/orderdb.c
@@ -1,7 +1,6 @@
 #include <platform.h>
 
 #include "kernel/config.h"
-#include "kernel/db/driver.h"
 
 #include "orderdb.h"
 
@@ -13,30 +12,3 @@
 #include <stdlib.h>
 #include <string.h>
 
-void orderdb_open(void)
-{
-    const char *dbname;
-    dbname = config_get("game.dbswap");
-    db_driver_open(DB_SWAP, dbname);
-}
-
-void orderdb_close(void)
-{
-    db_driver_close(DB_SWAP);
-}
-
-order_data *odata_load(int id)
-{
-    if (id > 0) {
-        return db_driver_order_load(id);
-    }
-    return NULL;
-}
-
-int odata_save(order_data *od)
-{
-    if (od->_str) {
-        return db_driver_order_save(od->_str);
-    }
-    return 0;
-}

From 6738962d73941ad2eae07d58b2a52cd6ca295a97 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Wed, 24 Oct 2018 07:00:26 +0200
Subject: [PATCH 02/19] missing files

---
 src/kernel/database.c | 28 ++++++++++++++++++++++++++++
 src/kernel/database.h | 11 +++++++++++
 2 files changed, 39 insertions(+)
 create mode 100644 src/kernel/database.c
 create mode 100644 src/kernel/database.h

diff --git a/src/kernel/database.c b/src/kernel/database.c
new file mode 100644
index 000000000..fec8867a2
--- /dev/null
+++ b/src/kernel/database.c
@@ -0,0 +1,28 @@
+#include "config.h"
+#include "database.h"
+#include "db/driver.h"
+
+void swapdb_open(void)
+{
+    const char *dbname;
+    dbname = config_get("game.dbswap");
+    db_driver_open(DB_SWAP, dbname);
+}
+
+void swapdb_close(void)
+{
+    db_driver_close(DB_SWAP);
+}
+
+dbstring_id db_string_save(const char *s) {
+    (void)s;
+    return 0;
+}
+
+dbstring_id dbstring_save(const char *s) {
+    return db_driver_string_save(s);
+}
+
+const char *dbstring_load(dbstring_id id, size_t *size) {
+    return db_driver_string_load(id, size);
+}
diff --git a/src/kernel/database.h b/src/kernel/database.h
new file mode 100644
index 000000000..584b21908
--- /dev/null
+++ b/src/kernel/database.h
@@ -0,0 +1,11 @@
+#pragma once
+
+#include <stddef.h>
+
+typedef unsigned int dbstring_id;
+
+void swapdb_open(void);
+void swapdb_close(void);
+
+dbstring_id dbstring_save(const char *s);
+const char *dbstring_load(dbstring_id id, size_t *size);

From 83959c375f9c667812c0948980fbad800ffd1d05 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno@eressea.de>
Date: Wed, 24 Oct 2018 09:02:19 +0200
Subject: [PATCH 03/19] kill orderdb.[hc] files

---
 src/CMakeLists.txt         |  2 --
 src/eressea.c              |  1 -
 src/kernel/database.c      |  7 ++++---
 src/kernel/database.h      |  8 +++----
 src/kernel/database.test.c | 14 ++++++++++++-
 src/kernel/db/driver.h     | 12 ++++++-----
 src/kernel/db/sqlite.c     | 22 +++++++++----------
 src/kernel/orderdb.c       | 43 --------------------------------------
 src/kernel/orderdb.h       | 16 --------------
 src/orderdb.c              | 14 -------------
 src/orderdb.h              | 25 ----------------------
 src/orderdb.test.c         | 28 -------------------------
 src/test_eressea.c         |  1 -
 13 files changed, 39 insertions(+), 154 deletions(-)
 delete mode 100644 src/kernel/orderdb.c
 delete mode 100644 src/kernel/orderdb.h
 delete mode 100644 src/orderdb.c
 delete mode 100644 src/orderdb.h
 delete mode 100644 src/orderdb.test.c

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 4e8db8bf9..c42e3eb2e 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -117,7 +117,6 @@ set (ERESSEA_SRC
   morale.c
   move.c
   names.c
-  orderdb.c
   orderfile.c
   piracy.c
   prefix.c
@@ -230,7 +229,6 @@ set(TESTS_SRC
   monsters.test.c
   move.test.c
   names.test.c
-  orderdb.test.c
   orderfile.test.c
   piracy.test.c
   prefix.test.c
diff --git a/src/eressea.c b/src/eressea.c
index 1221f93e0..8e7da7f7a 100644
--- a/src/eressea.c
+++ b/src/eressea.c
@@ -28,7 +28,6 @@
 #include "creport.h"
 #include "report.h"
 #include "names.h"
-#include "orderdb.h"
 #include "reports.h"
 #include "spells.h"
 #include "vortex.h"
diff --git a/src/kernel/database.c b/src/kernel/database.c
index fec8867a2..796aa2380 100644
--- a/src/kernel/database.c
+++ b/src/kernel/database.c
@@ -14,15 +14,16 @@ void swapdb_close(void)
     db_driver_close(DB_SWAP);
 }
 
-dbstring_id db_string_save(const char *s) {
+dbrow_id db_string_save(const char *s) {
     (void)s;
     return 0;
 }
 
-dbstring_id dbstring_save(const char *s) {
+dbrow_id dbstring_save(const char *s) {
     return db_driver_string_save(s);
 }
 
-const char *dbstring_load(dbstring_id id, size_t *size) {
+const char *dbstring_load(dbrow_id id, size_t *size) {
     return db_driver_string_load(id, size);
 }
+
diff --git a/src/kernel/database.h b/src/kernel/database.h
index 584b21908..8147ccc61 100644
--- a/src/kernel/database.h
+++ b/src/kernel/database.h
@@ -1,11 +1,11 @@
 #pragma once
 
-#include <stddef.h>
+#include "db/driver.h"
 
-typedef unsigned int dbstring_id;
+#include <stddef.h>
 
 void swapdb_open(void);
 void swapdb_close(void);
 
-dbstring_id dbstring_save(const char *s);
-const char *dbstring_load(dbstring_id id, size_t *size);
+dbrow_id dbstring_save(const char *s);
+const char *dbstring_load(dbrow_id id, size_t *size);
diff --git a/src/kernel/database.test.c b/src/kernel/database.test.c
index 3ec44896e..04108aa85 100644
--- a/src/kernel/database.test.c
+++ b/src/kernel/database.test.c
@@ -3,8 +3,8 @@
 #include <kernel/faction.h>
 #include <kernel/order.h>
 
+#include "database.h"
 #include "db/driver.h"
-#include "orderdb.h"
 
 #include <CuTest.h>
 #include <tests.h>
@@ -12,6 +12,17 @@
 #include <stdio.h>
 #include <string.h>
 
+static void test_orderdb(CuTest *tc) {
+    order_data *od = NULL;
+    const char * s = "GIB enno 1 Hodor";
+
+    odata_create(&od, strlen(s) + 1, s);
+    CuAssertPtrNotNull(tc, od);
+    CuAssertStrEquals(tc, s, od->_str);
+    CuAssertTrue(tc, od->_refcount >= 1);
+    odata_release(od);
+}
+
 static void test_save_load_order(CuTest *tc) {
     order_data *od;
     int id;
@@ -52,6 +63,7 @@ CuSuite *get_db_suite(void)
     CuSuite *suite = CuSuiteNew();
     SUITE_ADD_TEST(suite, test_save_load_order);
     SUITE_ADD_TEST(suite, test_update_faction);
+    SUITE_ADD_TEST(suite, test_orderdb);
 
     return suite;
 }
diff --git a/src/kernel/db/driver.h b/src/kernel/db/driver.h
index e127c6662..341653991 100644
--- a/src/kernel/db/driver.h
+++ b/src/kernel/db/driver.h
@@ -4,6 +4,8 @@
 
 struct order_data;
 
+typedef unsigned int dbrow_id;
+
 extern void odata_create(struct order_data **pdata, size_t len, const char *str);
 
 typedef enum database_t {
@@ -13,8 +15,8 @@ typedef enum database_t {
 
 int db_driver_open(database_t db, const char *dbname);
 void db_driver_close(database_t db);
-int db_driver_order_save(const char *str);
-struct order_data *db_driver_order_load(int id);
-int db_driver_faction_save(int id, int no, int turn, const char *email, const char *password);
-unsigned int db_driver_string_save(const char *s);
-const char *db_driver_string_load(unsigned int id, size_t *size);
+dbrow_id db_driver_order_save(const char *str);
+struct order_data *db_driver_order_load(dbrow_id id);
+dbrow_id db_driver_faction_save(dbrow_id id, int no, int turn, const char *email, const char *password);
+dbrow_id db_driver_string_save(const char *s);
+const char *db_driver_string_load(dbrow_id id, size_t *size);
diff --git a/src/kernel/db/sqlite.c b/src/kernel/db/sqlite.c
index 9df27f365..9a4dbc9ff 100644
--- a/src/kernel/db/sqlite.c
+++ b/src/kernel/db/sqlite.c
@@ -35,7 +35,7 @@ static void end_transaction(void) {
     }
 }
 
-struct order_data *db_driver_order_load(int id)
+struct order_data *db_driver_order_load(dbrow_id id)
 {
     struct order_data * od = NULL;
     int err;
@@ -61,7 +61,7 @@ struct order_data *db_driver_order_load(int id)
     return NULL;
 }
 
-int db_driver_order_save(const char *str) {
+dbrow_id db_driver_order_save(const char *str) {
     int err;
     sqlite3_int64 id;
     
@@ -83,7 +83,7 @@ int db_driver_order_save(const char *str) {
     err = sqlite3_step(g_stmt_insert_order);
     assert(err == SQLITE_DONE);
     id = sqlite3_last_insert_rowid(g_swap_db);
-    assert(id <= INT_MAX);
+    assert(id > 0 && id <= UINT_MAX);
     
     if (g_insert_batchsize > 0) {
         if (++g_insert_tx_size >= g_insert_batchsize) {
@@ -91,11 +91,11 @@ int db_driver_order_save(const char *str) {
         }
     }
     ERRNO_CHECK(); 
-    return (int)id;
+    return (dbrow_id)id;
 }
 
 
-int db_driver_faction_save(int id, int no, int turn, const char *email, const char *password)
+dbrow_id db_driver_faction_save(dbrow_id id, int no, int turn, const char *email, const char *password)
 {
     sqlite3_int64 row_id;
     int err;
@@ -140,8 +140,8 @@ int db_driver_faction_save(int id, int no, int turn, const char *email, const ch
     ERRNO_CHECK();
 
     row_id = sqlite3_last_insert_rowid(g_game_db);
-    assert(row_id <= INT_MAX);
-    return (int)row_id;
+    assert(row_id>0 && row_id <= UINT_MAX);
+    return (dbrow_id)row_id;
 }
 
 static int db_open_game(const char *dbname) {
@@ -240,7 +240,7 @@ void db_driver_close(database_t db)
     ERRNO_CHECK();
 }
 
-unsigned int db_driver_string_save(const char *str) {
+dbrow_id db_driver_string_save(const char *str) {
     int err;
     sqlite3_int64 id;
 
@@ -262,7 +262,7 @@ unsigned int db_driver_string_save(const char *str) {
     err = sqlite3_step(g_stmt_insert_string);
     assert(err == SQLITE_DONE);
     id = sqlite3_last_insert_rowid(g_swap_db);
-    assert(id <= INT_MAX);
+    assert(id > 0 && id <= UINT_MAX);
 
     if (g_insert_batchsize > 0) {
         if (++g_insert_tx_size >= g_insert_batchsize) {
@@ -270,10 +270,10 @@ unsigned int db_driver_string_save(const char *str) {
         }
     }
     ERRNO_CHECK();
-    return (int)id;
+    return (dbrow_id)id;
 }
 
-const char *db_driver_string_load(unsigned int id, size_t *size) {
+const char *db_driver_string_load(dbrow_id id, size_t *size) {
     int err;
 
     end_transaction();
diff --git a/src/kernel/orderdb.c b/src/kernel/orderdb.c
deleted file mode 100644
index 42248c0ef..000000000
--- a/src/kernel/orderdb.c
+++ /dev/null
@@ -1,43 +0,0 @@
-#include <platform.h>
-
-#include "kernel/config.h"
-#include "kernel/db/driver.h"
-
-#include "orderdb.h"
-
-#include <util/log.h>
-
-#include <critbit.h>
-
-#include <assert.h>
-#include <stdlib.h>
-#include <string.h>
-
-void orderdb_open(void)
-{
-    const char *dbname;
-
-    dbname = config_get("game.dbswap");
-    db_driver_open(DB_SWAP, dbname);
-}
-
-void orderdb_close(void)
-{
-    db_driver_close(DB_SWAP);
-}
-
-order_data *odata_load(int id)
-{
-    if (id > 0) {
-        return db_driver_order_load(id);
-    }
-    return NULL;
-}
-
-int odata_save(order_data *od)
-{
-    if (od->_str) {
-        return db_driver_order_save(od->_str);
-    }
-    return 0;
-}
diff --git a/src/kernel/orderdb.h b/src/kernel/orderdb.h
deleted file mode 100644
index e17bdfea8..000000000
--- a/src/kernel/orderdb.h
+++ /dev/null
@@ -1,16 +0,0 @@
-#ifndef H_ORDERDB
-#define H_ORDERDB
-
-#include <stddef.h>
-
-#ifdef __cplusplus
-extern "C" {
-#endif
-
-    void orderdb_open(void);
-    void orderdb_close(void);
-
-#ifdef __cplusplus
-}
-#endif
-#endif
diff --git a/src/orderdb.c b/src/orderdb.c
deleted file mode 100644
index 87b6e626c..000000000
--- a/src/orderdb.c
+++ /dev/null
@@ -1,14 +0,0 @@
-#include <platform.h>
-
-#include "kernel/config.h"
-
-#include "orderdb.h"
-
-#include <util/log.h>
-
-#include <critbit.h>
-
-#include <assert.h>
-#include <stdlib.h>
-#include <string.h>
-
diff --git a/src/orderdb.h b/src/orderdb.h
deleted file mode 100644
index cc809b1a6..000000000
--- a/src/orderdb.h
+++ /dev/null
@@ -1,25 +0,0 @@
-#ifndef H_ORDERDB
-#define H_ORDERDB
-
-#include <stddef.h>
-
-#ifdef __cplusplus
-extern "C" {
-#endif
-
-    typedef struct order_data {
-        const char *_str;
-        int _refcount;
-    } order_data;
-
-    void odata_create(order_data **pdata, size_t len, const char *str);
-    void odata_release(order_data * od);
-    void odata_addref(order_data *od);
-
-    void orderdb_open(void);
-    void orderdb_close(void);
-
-#ifdef __cplusplus
-}
-#endif
-#endif
diff --git a/src/orderdb.test.c b/src/orderdb.test.c
deleted file mode 100644
index bdb6f0a95..000000000
--- a/src/orderdb.test.c
+++ /dev/null
@@ -1,28 +0,0 @@
-#include <platform.h>
-#include <kernel/config.h>
-
-#include "orderdb.h"
-
-#include <CuTest.h>
-#include <tests.h>
-
-#include <string.h>
-
-static void test_orderdb(CuTest *tc) {
-    order_data *od = NULL;
-    const char * s = "GIB enno 1 Hodor";
-
-    odata_create(&od, strlen(s) + 1, s);
-    CuAssertPtrNotNull(tc, od);
-    CuAssertStrEquals(tc, s, od->_str);
-    CuAssertTrue(tc, od->_refcount >= 1);
-    odata_release(od);
-}
-
-CuSuite *get_orderdb_suite(void)
-{
-    CuSuite *suite = CuSuiteNew();
-    SUITE_ADD_TEST(suite, test_orderdb);
-
-    return suite;
-}
diff --git a/src/test_eressea.c b/src/test_eressea.c
index 2b36d4e03..12c658f91 100644
--- a/src/test_eressea.c
+++ b/src/test_eressea.c
@@ -138,7 +138,6 @@ int RunAllTests(int argc, char *argv[])
     ADD_SUITE(monsters);
     ADD_SUITE(move);
     ADD_SUITE(names);
-    ADD_SUITE(orderdb);
     ADD_SUITE(orderfile);
     ADD_SUITE(otherfaction);
     ADD_SUITE(piracy);

From 32009b70439bcfa5f446428b13e31dcc5885f1c0 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno@eressea.de>
Date: Wed, 24 Oct 2018 09:27:48 +0200
Subject: [PATCH 04/19] store passwords in swapdb

---
 src/bind_faction.c         |  2 +-
 src/gamedb.c               |  4 +++-
 src/kernel/database.test.c |  8 ++++++--
 src/kernel/faction.c       | 21 +++++++++++++++------
 src/kernel/faction.h       |  4 +++-
 src/kernel/save.c          |  4 +---
 src/kernel/save.test.c     |  2 +-
 src/reports.c              |  8 --------
 src/reports.test.c         |  2 +-
 9 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/src/bind_faction.c b/src/bind_faction.c
index bb0920395..46967252c 100644
--- a/src/bind_faction.c
+++ b/src/bind_faction.c
@@ -432,7 +432,7 @@ static int tolua_faction_create(lua_State * L)
 static int tolua_faction_get_password(lua_State * L)
 {
     faction *self = (faction *)tolua_tousertype(L, 1, 0);
-    tolua_pushstring(L, self->_password);
+    tolua_pushstring(L, faction_getpassword(self));
     return 1;
 }
 
diff --git a/src/gamedb.c b/src/gamedb.c
index 2ca2dba73..5f53d9120 100644
--- a/src/gamedb.c
+++ b/src/gamedb.c
@@ -20,7 +20,9 @@ int gamedb_update(void)
     err = db_driver_open(DB_GAME, dbname);
     if (err == 0) {
         for (f = factions; f; f = f->next) {
-            int uid = db_driver_faction_save(f->uid, f->no, turn, f->email, f->_password);
+            int uid = db_driver_faction_save(f->uid, f->no, turn,
+                faction_getemail(f),
+                faction_getpassword(f));
             if (uid > 0) {
                 f->uid = uid;
             }
diff --git a/src/kernel/database.test.c b/src/kernel/database.test.c
index 04108aa85..8be957cb4 100644
--- a/src/kernel/database.test.c
+++ b/src/kernel/database.test.c
@@ -51,9 +51,13 @@ static void test_update_faction(CuTest *tc) {
 
     test_setup();
     f = test_create_faction(NULL);
-    uid = db_driver_faction_save(f->uid, f->no, 0, f->email, f->_password);
+    uid = db_driver_faction_save(f->uid, f->no, 0,
+        faction_getemail(f),
+        faction_getpassword(f));
     f->uid = uid;
-    uid = db_driver_faction_save(f->uid, f->no, 0, f->email, f->_password);
+    uid = db_driver_faction_save(f->uid, f->no, 0,
+        faction_getemail(f),
+        faction_getpassword(f));
     CuAssertIntEquals(tc, f->uid, uid);
     test_teardown();
 }
diff --git a/src/kernel/faction.c b/src/kernel/faction.c
index 21ea35833..a3cd96bc9 100755
--- a/src/kernel/faction.c
+++ b/src/kernel/faction.c
@@ -22,6 +22,7 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
 #include "calendar.h"
 #include "config.h"
+#include "database.h"
 #include "alliance.h"
 #include "ally.h"
 #include "curse.h"
@@ -106,7 +107,6 @@ static void free_faction(faction * f)
 
     free(f->email);
     free(f->banner);
-    free(f->_password);
     free(f->name);
     if (f->seen_factions) {
         selist_free(f->seen_factions);
@@ -245,7 +245,7 @@ faction *addfaction(const char *email, const char *password,
     f->alliance_joindate = turn;
     f->lastorders = turn;
     f->_alive = true;
-    f->_password = NULL;
+    f->password_id = 0;
     f->age = 0;
     f->race = frace;
     f->magiegebiet = 0;
@@ -321,9 +321,11 @@ unit *addplayer(region * r, faction * f)
 
 bool checkpasswd(const faction * f, const char *passwd)
 {
+    const char *pwhash;
     if (!passwd) return false;
 
-    if (f->_password && password_verify(f->_password, passwd) == VERIFY_FAIL) {
+    pwhash = faction_getpassword(f);
+    if (pwhash && password_verify(pwhash, passwd) == VERIFY_FAIL) {
         log_info("password check failed: %s", factionname(f));
         return false;
     }
@@ -603,11 +605,17 @@ void faction_setbanner(faction * self, const char *banner)
         self->banner = str_strdup(banner);
 }
 
+const char *faction_getpassword(const faction *f) {
+    if (f->password_id > 0) {
+        return dbstring_load(f->password_id, NULL);
+    }
+    return NULL;
+}
+
 void faction_setpassword(faction * f, const char *pwhash)
 {
     assert(pwhash);
-    free(f->_password);
-    f->_password = str_strdup(pwhash);
+    f->password_id = dbstring_save(pwhash);
 }
 
 bool valid_race(const struct faction *f, const struct race *rc)
@@ -844,7 +852,8 @@ int writepasswd(void)
 
         for (f = factions; f; f = f->next) {
             fprintf(F, "%s:%s:%s:%d\n",
-                itoa36(f->no), faction_getemail(f), f->_password, f->uid);
+                itoa36(f->no), faction_getemail(f),
+               faction_getpassword(f), f->uid);
         }
         fclose(F);
         return 0;
diff --git a/src/kernel/faction.h b/src/kernel/faction.h
index 0fe38cb1d..8548328ef 100644
--- a/src/kernel/faction.h
+++ b/src/kernel/faction.h
@@ -21,6 +21,7 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
 #include "skill.h"
 #include "types.h"
+#include "db/driver.h"
 
 #include <util/resolve.h>
 #include <modules/score.h>
@@ -72,7 +73,7 @@ extern "C" {
         char *name;
         char *banner;
         char *email;
-        char *_password;
+        dbrow_id password_id;
         int max_spelllevel;
         struct spellbook *spellbook;
         const struct locale *locale;
@@ -154,6 +155,7 @@ extern "C" {
     void faction_setemail(struct faction *self, const char *email);
 
     void faction_setpassword(struct faction *self, const char *pwhash);
+    const char *faction_getpassword(const struct faction *f);
     bool valid_race(const struct faction *f, const struct race *rc);
 
     void faction_getorigin(const struct faction * f, int id, int *x, int *y);
diff --git a/src/kernel/save.c b/src/kernel/save.c
index a61e4348f..63e14b3c3 100644
--- a/src/kernel/save.c
+++ b/src/kernel/save.c
@@ -931,7 +931,6 @@ static void read_password(gamedata *data, faction *f) {
     else {
         faction_setpassword(f, (data->version >= CRYPT_VERSION) ? name : password_hash(name, PASSWORD_DEFAULT));
     }
-    (void)_test_read_password;
 }
 
 void _test_read_password(gamedata *data, faction *f) {
@@ -939,8 +938,7 @@ void _test_read_password(gamedata *data, faction *f) {
 }
 
 static void write_password(gamedata *data, const faction *f) {
-    WRITE_TOK(data->store, (const char *)f->_password);
-    (void)_test_write_password;
+    WRITE_TOK(data->store, faction_getpassword(f));
 }
 
 void _test_write_password(gamedata *data, const faction *f) {
diff --git a/src/kernel/save.test.c b/src/kernel/save.test.c
index 4844d1189..a70a27c2f 100644
--- a/src/kernel/save.test.c
+++ b/src/kernel/save.test.c
@@ -432,7 +432,7 @@ static void test_read_password_external(CuTest *tc) {
     }
     f = test_create_faction(NULL);
     faction_setpassword(f, password_hash("secret", PASSWORD_DEFAULT));
-    CuAssertPtrNotNull(tc, f->_password);
+    CuAssertPtrNotNull(tc, faction_getpassword(f));
     mstream_init(&data.strm);
     gamedata_init(&data, &store, RELEASE_VERSION);
     WRITE_TOK(data.store, "newpassword");
diff --git a/src/reports.c b/src/reports.c
index 879e7ad3d..8088af347 100644
--- a/src/reports.c
+++ b/src/reports.c
@@ -1554,14 +1554,6 @@ void prepare_report(report_context *ctx, faction *f)
         rule_lighthouse_units = config_get_int("rules.lighthouse.unit_capacity", 0) != 0;
     }
 
-    if (f->age <= 2) {
-        if ((f->flags&FFL_PWMSG) == 0) {
-            /* TODO: this assumes unencrypted passwords */
-            f->flags |= FFL_PWMSG;
-            ADDMSG(&f->msgs, msg_message("changepasswd", "value", f->_password));
-        }
-    }
-
     ctx->f = f;
     ctx->report_time = time(NULL);
     ctx->addresses = NULL;
diff --git a/src/reports.test.c b/src/reports.test.c
index d402b2b6a..e7fcc282b 100644
--- a/src/reports.test.c
+++ b/src/reports.test.c
@@ -902,7 +902,7 @@ CuSuite *get_reports_suite(void)
     SUITE_ADD_TEST(suite, test_region_distance);
     SUITE_ADD_TEST(suite, test_region_distance_max);
     SUITE_ADD_TEST(suite, test_region_distance_ql);
-    SUITE_ADD_TEST(suite, test_newbie_password_message);
+    DISABLE_TEST(suite, test_newbie_password_message);
     SUITE_ADD_TEST(suite, test_prepare_report);
     SUITE_ADD_TEST(suite, test_seen_neighbours);
     SUITE_ADD_TEST(suite, test_seen_travelthru);

From da3270517a742aabad5754f3c9eed943829e38e2 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Wed, 24 Oct 2018 19:39:30 +0200
Subject: [PATCH 05/19] fix type conversion and incomplete database test.

---
 src/kernel/database.test.c |  2 ++
 src/kernel/db/sqlite.c     | 10 ++++++----
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/kernel/database.test.c b/src/kernel/database.test.c
index 8be957cb4..c5aae1af6 100644
--- a/src/kernel/database.test.c
+++ b/src/kernel/database.test.c
@@ -50,6 +50,7 @@ static void test_update_faction(CuTest *tc) {
     int uid;
 
     test_setup();
+    db_driver_open(DB_GAME, NULL);
     f = test_create_faction(NULL);
     uid = db_driver_faction_save(f->uid, f->no, 0,
         faction_getemail(f),
@@ -59,6 +60,7 @@ static void test_update_faction(CuTest *tc) {
         faction_getemail(f),
         faction_getpassword(f));
     CuAssertIntEquals(tc, f->uid, uid);
+    db_driver_close(DB_GAME);
     test_teardown();
 }
 
diff --git a/src/kernel/db/sqlite.c b/src/kernel/db/sqlite.c
index 9a4dbc9ff..6aff52cf1 100644
--- a/src/kernel/db/sqlite.c
+++ b/src/kernel/db/sqlite.c
@@ -100,9 +100,7 @@ dbrow_id db_driver_faction_save(dbrow_id id, int no, int turn, const char *email
     sqlite3_int64 row_id;
     int err;
 
-    if (!g_game_db) {
-        return -1;
-    }
+    assert(g_game_db);
     if (id != 0) {
         int rows;
 
@@ -192,7 +190,11 @@ static const char *g_swapname;
 int db_driver_open(database_t db, const char *dbname)
 {
     ERRNO_CHECK();
-
+    
+    if (!dbname) {
+        /* by default, use an in-memory database */
+        dbname = ":memory:";
+    }
     if (db == DB_SWAP) {
         g_swapname = dbname;
         return db_open_swap(dbname);

From 516a53c0c7da706c604734f3a2d44e1333ecccd4 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Wed, 24 Oct 2018 19:54:07 +0200
Subject: [PATCH 06/19] swap faction.banner to database

---
 src/creport.c             | 15 ++++++++++-----
 src/kernel/faction.c      | 14 +++++++-------
 src/kernel/faction.h      |  2 +-
 src/kernel/faction.test.c | 15 ++++++++++++++-
 src/kernel/save.c         |  4 ++--
 src/laws.c                |  6 ++----
 src/report.c              |  4 +++-
 7 files changed, 39 insertions(+), 21 deletions(-)

diff --git a/src/creport.c b/src/creport.c
index 475a04b1e..0173b7409 100644
--- a/src/creport.c
+++ b/src/creport.c
@@ -1057,12 +1057,15 @@ static void cr_find_address(FILE * F, const faction * uf, selist * addresses)
     while (flist) {
         const faction *f = (const faction *)selist_get(flist, i);
         if (uf != f) {
+            const char *str;
             fprintf(F, "PARTEI %d\n", f->no);
             fprintf(F, "\"%s\";Parteiname\n", f->name);
             if (strcmp(faction_getemail(f), "") != 0)
                 fprintf(F, "\"%s\";email\n", faction_getemail(f));
-            if (f->banner)
-                fprintf(F, "\"%s\";banner\n", f->banner);
+            str = faction_getbanner(f);
+            if (str) {
+                fprintf(F, "\"%s\";banner\n", str);
+            }
             fprintf(F, "\"%s\";locale\n", locale_name(f->locale));
             if (f->alliance && f->alliance == uf->alliance) {
                 fprintf(F, "%d;alliance\n", f->alliance->id);
@@ -1534,7 +1537,7 @@ report_computer(const char *filename, report_context * ctx, const char *bom)
     static int era = -1;
     int i;
     faction *f = ctx->f;
-    const char *prefix;
+    const char *prefix, *str;
     region *r;
     const char *mailto = config_get("game.email");
     const attrib *a;
@@ -1628,8 +1631,10 @@ report_computer(const char *filename, report_context * ctx, const char *bom)
 
     fprintf(F, "\"%s\";Parteiname\n", f->name);
     fprintf(F, "\"%s\";email\n", faction_getemail(f));
-    if (f->banner)
-        fprintf(F, "\"%s\";banner\n", f->banner);
+    str = faction_getbanner(f);
+    if (str) {
+        fprintf(F, "\"%s\";banner\n", str);
+    }
     print_items(F, f->items, f->locale);
     fputs("OPTIONEN\n", F);
     for (i = 0; i != MAXOPTIONS; ++i) {
diff --git a/src/kernel/faction.c b/src/kernel/faction.c
index a3cd96bc9..bb5a27271 100755
--- a/src/kernel/faction.c
+++ b/src/kernel/faction.c
@@ -106,7 +106,6 @@ static void free_faction(faction * f)
     freelist(f->allies);
 
     free(f->email);
-    free(f->banner);
     free(f->name);
     if (f->seen_factions) {
         selist_free(f->seen_factions);
@@ -593,16 +592,17 @@ void faction_setemail(faction * self, const char *email)
         self->email = NULL;
 }
 
-const char *faction_getbanner(const faction * self)
+const char *faction_getbanner(const faction * f)
 {
-    return self->banner ? self->banner : "";
+    if (f->banner_id > 0) {
+        return dbstring_load(f->banner_id, NULL);
+    }
+    return NULL;
 }
 
-void faction_setbanner(faction * self, const char *banner)
+void faction_setbanner(faction * f, const char *banner)
 {
-    free(self->banner);
-    if (banner)
-        self->banner = str_strdup(banner);
+    f->banner_id = dbstring_save(banner);
 }
 
 const char *faction_getpassword(const faction *f) {
diff --git a/src/kernel/faction.h b/src/kernel/faction.h
index 8548328ef..c10e6ec69 100644
--- a/src/kernel/faction.h
+++ b/src/kernel/faction.h
@@ -71,7 +71,7 @@ extern "C" {
         int uid;
         int flags;
         char *name;
-        char *banner;
+        dbrow_id banner_id;
         char *email;
         dbrow_id password_id;
         int max_spelllevel;
diff --git a/src/kernel/faction.test.c b/src/kernel/faction.test.c
index 6c2ed3aa6..9aac0465c 100644
--- a/src/kernel/faction.test.c
+++ b/src/kernel/faction.test.c
@@ -117,7 +117,7 @@ static void test_addfaction(CuTest *tc) {
     CuAssertPtrNotNull(tc, f->name);
     CuAssertPtrEquals(tc, NULL, (void *)f->units);
     CuAssertPtrEquals(tc, NULL, (void *)f->next);
-    CuAssertPtrEquals(tc, NULL, (void *)f->banner);
+    CuAssertPtrEquals(tc, NULL, (void *)faction_getbanner(f));
     CuAssertPtrEquals(tc, NULL, (void *)f->spellbook);
     CuAssertPtrEquals(tc, NULL, (void *)f->origin);
     CuAssertPtrEquals(tc, (void *)factions, (void *)f);
@@ -229,6 +229,18 @@ static void test_valid_race(CuTest *tc) {
     test_teardown();
 }
 
+static void test_dbstrings(CuTest *tc) {
+    const char *lipsum = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.";
+    faction *f;
+    test_setup();
+    f = test_create_faction(NULL);
+    faction_setbanner(f, lipsum);
+    faction_setpassword(f, lipsum + 12);
+    CuAssertStrEquals(tc, lipsum, faction_getbanner(f));
+    CuAssertStrEquals(tc, lipsum + 12, faction_getpassword(f));
+    test_teardown();
+}
+
 static void test_set_email(CuTest *tc) {
     faction *f;
     char email[10];
@@ -335,6 +347,7 @@ CuSuite *get_faction_suite(void)
     SUITE_ADD_TEST(suite, test_check_passwd);
     SUITE_ADD_TEST(suite, test_valid_race);
     SUITE_ADD_TEST(suite, test_set_email);
+    SUITE_ADD_TEST(suite, test_dbstrings);
     SUITE_ADD_TEST(suite, test_save_special_items);
     return suite;
 }
diff --git a/src/kernel/save.c b/src/kernel/save.c
index 63e14b3c3..f1b614709 100644
--- a/src/kernel/save.c
+++ b/src/kernel/save.c
@@ -1007,7 +1007,7 @@ faction *read_faction(gamedata * data)
 	if (unicode_utf8_trim(name)!=0) {
 		log_warning("trim faction %s banner to '%s'", itoa36(f->no), name);
 	};
-    f->banner = str_strdup(name);
+    faction_setbanner(f, name);
 
     log_debug("   - Lese Partei %s (%s)", f->name, itoa36(f->no));
 
@@ -1115,7 +1115,7 @@ void write_faction(gamedata *data, const faction * f)
     WRITE_INT(data->store, f->alliance_joindate);
 
     WRITE_STR(data->store, f->name);
-    WRITE_STR(data->store, f->banner);
+    WRITE_STR(data->store, faction_getbanner(f));
     WRITE_STR(data->store, f->email?f->email:"");
     write_password(data, f);
     WRITE_TOK(data->store, locale_name(f->locale));
diff --git a/src/laws.c b/src/laws.c
index 4c2a29e4e..4212f9d63 100644
--- a/src/laws.c
+++ b/src/laws.c
@@ -2086,12 +2086,10 @@ int banner_cmd(unit * u, struct order *ord)
 {
     const char * s;
 
-    free(u->faction->banner);
     init_order_depr(ord);
     s = getstrtoken();
-    u->faction->banner = s ? str_strdup(s) : 0;
-    add_message(&u->faction->msgs, msg_message("changebanner", "value",
-        u->faction->banner));
+    faction_setbanner(u->faction, s);
+    add_message(&u->faction->msgs, msg_message("changebanner", "value", s));
 
     return 0;
 }
diff --git a/src/report.c b/src/report.c
index 5d2ed4598..f744086c3 100644
--- a/src/report.c
+++ b/src/report.c
@@ -1726,11 +1726,13 @@ static void list_address(struct stream *out, const faction * uf, selist * seenfa
     while (flist != NULL) {
         const faction *f = (const faction *)selist_get(flist, qi);
         if (!is_monsters(f)) {
+            const char *str;
             char buf[8192];
             char label = '-';
 
+            str = faction_getbanner(f);
             sprintf(buf, "%s: %s; %s", factionname(f), faction_getemail(f),
-                f->banner ? f->banner : "");
+                str ? str : "");
             if (uf == f)
                 label = '*';
             else if (is_allied(uf, f))

From 53e123b36aed63f4d6d8454b3a6bfe9cd98a0edf Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Wed, 24 Oct 2018 20:16:17 +0200
Subject: [PATCH 07/19] do not use unit.display directly, use getter/setter
 instead.

---
 src/kernel/save.c         |  8 +++++---
 src/kernel/unit.c         | 14 +++++++-------
 src/kernel/unit.h         |  2 +-
 src/kernel/unit.test.c    |  9 ++++-----
 src/laws.c                |  2 +-
 src/laws.test.c           |  4 ++--
 src/spells/combatspells.c |  7 +------
 7 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/src/kernel/save.c b/src/kernel/save.c
index f1b614709..b9f501a4e 100644
--- a/src/kernel/save.c
+++ b/src/kernel/save.c
@@ -440,12 +440,12 @@ unit *read_unit(gamedata *data)
     if (unicode_utf8_trim(obuf)!=0) {
 		log_warning("trim unit %s name to '%s'", itoa36(u->no), obuf);
 	}
-    u->_name = obuf[0] ? str_strdup(obuf) : 0;
+    unit_setname(u, obuf[0] ? obuf : NULL);
     READ_STR(data->store, obuf, sizeof(obuf));
     if (unicode_utf8_trim(obuf)!=0) {
         log_warning("trim unit %s info to '%s'", itoa36(u->no), obuf);
     }
-    u->display = obuf[0] ? str_strdup(obuf) : 0;
+    unit_setinfo(u, obuf[0] ? obuf : NULL);
     READ_INT(data->store, &number);
     set_number(u, number);
 
@@ -544,6 +544,7 @@ unit *read_unit(gamedata *data)
 
 void write_unit(gamedata *data, const unit * u)
 {
+    const char *str;
     order *ord;
     int p = 0;
     unsigned int flags = u->flags & UFL_SAVEMASK;
@@ -553,7 +554,8 @@ void write_unit(gamedata *data, const unit * u)
     assert(u->faction->_alive);
     write_faction_reference(u->faction, data->store);
     WRITE_STR(data->store, u->_name);
-    WRITE_STR(data->store, u->display ? u->display : "");
+    str = unit_getinfo(u);
+    WRITE_STR(data->store, str ? str : "");
     WRITE_INT(data->store, u->number);
     WRITE_INT(data->store, u->age);
     WRITE_TOK(data->store, u_race(u)->_name);
diff --git a/src/kernel/unit.c b/src/kernel/unit.c
index 9b6c3e9ee..d90c638a0 100644
--- a/src/kernel/unit.c
+++ b/src/kernel/unit.c
@@ -474,8 +474,8 @@ attrib_type at_private = {
 
 const char *u_description(const unit * u, const struct locale *lang)
 {
-    if (u->display && u->display[0]) {
-        return u->display;
+    if (u->_display && u->_display[0]) {
+        return u->_display;
     }
     else {
         char zText[64];
@@ -1293,7 +1293,7 @@ void free_unit(unit * u)
 {
     assert(!u->region);
     free(u->_name);
-    free(u->display);
+    free(u->_display);
     free_order(u->thisorder);
     free_orders(&u->orders);
     if (u->skills)
@@ -1522,16 +1522,16 @@ void unit_setname(unit * u, const char *name)
 
 const char *unit_getinfo(const unit * u)
 {
-    return (const char *)u->display;
+    return (const char *)u->_display;
 }
 
 void unit_setinfo(unit * u, const char *info)
 {
-    free(u->display);
+    free(u->_display);
     if (info)
-        u->display = str_strdup(info);
+        u->_display = str_strdup(info);
     else
-        u->display = NULL;
+        u->_display = NULL;
 }
 
 int unit_getid(const unit * u)
diff --git a/src/kernel/unit.h b/src/kernel/unit.h
index 3f82ef80a..582b99db3 100644
--- a/src/kernel/unit.h
+++ b/src/kernel/unit.h
@@ -89,7 +89,7 @@ extern "C" {
         int no;                     /* id */
         int hp;
         char *_name;
-        char *display;
+        char *_display;
         struct faction *faction;
         struct building *building;
         struct ship *ship;
diff --git a/src/kernel/unit.test.c b/src/kernel/unit.test.c
index 085e0177a..0918e83aa 100644
--- a/src/kernel/unit.test.c
+++ b/src/kernel/unit.test.c
@@ -397,14 +397,13 @@ static void test_unit_description(CuTest *tc) {
     rc = test_create_race("hodor");
     u = test_create_unit(test_create_faction(rc), test_create_region(0, 0, NULL));
 
-    CuAssertPtrEquals(tc, NULL, u->display);
-    CuAssertStrEquals(tc, 0, u_description(u, lang));
-    u->display = str_strdup("Hodor");
+    CuAssertStrEquals(tc, NULL, unit_getinfo(u));
+    CuAssertStrEquals(tc, NULL, u_description(u, lang));
+    unit_setinfo(u, "Hodor");
     CuAssertStrEquals(tc, "Hodor", u_description(u, NULL));
     CuAssertStrEquals(tc, "Hodor", u_description(u, lang));
 
-    free(u->display);
-    u->display = NULL;
+    unit_setinfo(u, NULL);
     locale_setstring(lang, "describe_hodor", "HODOR");
     CuAssertStrEquals(tc, "HODOR", u_description(u, lang));
 
diff --git a/src/laws.c b/src/laws.c
index 4212f9d63..6cbe7e726 100644
--- a/src/laws.c
+++ b/src/laws.c
@@ -1542,7 +1542,7 @@ int display_cmd(unit * u, struct order *ord)
         break;
 
     case P_UNIT:
-        s = &u->display;
+        unit_setinfo(u, getstrtoken());
         break;
 
     case P_PRIVAT:
diff --git a/src/laws.test.c b/src/laws.test.c
index fb8929996..524c750d4 100644
--- a/src/laws.test.c
+++ b/src/laws.test.c
@@ -188,12 +188,12 @@ static void test_display_cmd(CuTest *tc) {
 
     ord = create_order(K_DISPLAY, f->locale, "%s Hodor", LOC(f->locale, parameters[P_UNIT]));
     CuAssertIntEquals(tc, 0, display_cmd(u, ord));
-    CuAssertStrEquals(tc, "Hodor", u->display);
+    CuAssertStrEquals(tc, "Hodor", unit_getinfo(u));
     free_order(ord);
 
     ord = create_order(K_DISPLAY, f->locale, LOC(f->locale, parameters[P_UNIT]));
     CuAssertIntEquals(tc, 0, display_cmd(u, ord));
-    CuAssertPtrEquals(tc, NULL, u->display);
+    CuAssertPtrEquals(tc, NULL, (void *)unit_getinfo(u));
     free_order(ord);
 
     ord = create_order(K_DISPLAY, f->locale, "%s Hodor", LOC(f->locale, parameters[P_REGION]));
diff --git a/src/spells/combatspells.c b/src/spells/combatspells.c
index c088dde07..21b33e7bf 100644
--- a/src/spells/combatspells.c
+++ b/src/spells/combatspells.c
@@ -1539,12 +1539,7 @@ int sp_undeadhero(struct castorder * co)
 
             /* new units gets some stats from old unit */
 
-            if (du->display) {
-                unit_setinfo(u, du->display);
-            }
-            else {
-                unit_setinfo(u, NULL);
-            }
+            unit_setinfo(u, unit_getinfo(du));
             unit_setstatus(u, du->status);
             setguard(u, false);
             for (ilist = &du->items; *ilist;) {

From be0c0161da724ed09e467b1892d690da8f19e354 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Wed, 24 Oct 2018 20:21:21 +0200
Subject: [PATCH 08/19] use swap database for unit descriptions.

---
 src/kernel/unit.c | 21 ++++++++++++---------
 src/kernel/unit.h |  3 ++-
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/src/kernel/unit.c b/src/kernel/unit.c
index d90c638a0..b5346e04d 100644
--- a/src/kernel/unit.c
+++ b/src/kernel/unit.c
@@ -474,8 +474,8 @@ attrib_type at_private = {
 
 const char *u_description(const unit * u, const struct locale *lang)
 {
-    if (u->_display && u->_display[0]) {
-        return u->_display;
+    if (u->display_id > 0) {
+        return unit_getinfo(u);
     }
     else {
         char zText[64];
@@ -1293,7 +1293,6 @@ void free_unit(unit * u)
 {
     assert(!u->region);
     free(u->_name);
-    free(u->_display);
     free_order(u->thisorder);
     free_orders(&u->orders);
     if (u->skills)
@@ -1522,16 +1521,20 @@ void unit_setname(unit * u, const char *name)
 
 const char *unit_getinfo(const unit * u)
 {
-    return (const char *)u->_display;
+    if (u->display_id > 0) {
+        return dbstring_load(u->display_id, NULL);
+    }
+    return NULL;
 }
 
 void unit_setinfo(unit * u, const char *info)
 {
-    free(u->_display);
-    if (info)
-        u->_display = str_strdup(info);
-    else
-        u->_display = NULL;
+    if (info) {
+        u->display_id = dbstring_save(info);
+    }
+    else {
+        u->display_id = 0;
+    }
 }
 
 int unit_getid(const unit * u)
diff --git a/src/kernel/unit.h b/src/kernel/unit.h
index 582b99db3..a0f2f5a4c 100644
--- a/src/kernel/unit.h
+++ b/src/kernel/unit.h
@@ -21,6 +21,7 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
 #include <util/resolve.h>
 #include "types.h"
+#include "database.h"
 #include "skills.h"
 #include <stddef.h>
 
@@ -89,7 +90,7 @@ extern "C" {
         int no;                     /* id */
         int hp;
         char *_name;
-        char *_display;
+        dbrow_id display_id;
         struct faction *faction;
         struct building *building;
         struct ship *ship;

From c3d9e9dbaffc1733022111a99fef6fc89020add9 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Fri, 26 Oct 2018 14:13:00 +0200
Subject: [PATCH 09/19] report allies without bsdstring

---
 src/report.c | 133 +++++++++++++++++++++++----------------------------
 1 file changed, 59 insertions(+), 74 deletions(-)

diff --git a/src/report.c b/src/report.c
index 5d2ed4598..abad4c4fb 100644
--- a/src/report.c
+++ b/src/report.c
@@ -1514,93 +1514,78 @@ static void
 show_allies(const faction * f, const ally * allies, char *buf, size_t size)
 {
     int allierte = 0;
-    int i = 0, h, hh = 0;
-    int bytes, dh = 0;
+    int i = 0, h, hh = 0, dh = 0;
     const ally *sf;
-    char *bufp = buf;             /* buf already contains data */
-
-    --size;                       /* leave room for a null-terminator */
-
+   
     for (sf = allies; sf; sf = sf->next) {
         int mode = alliedgroup(NULL, f, sf->faction, sf, HELP_ALL);
-        if (mode > 0)
+        if (mode > 0) {
             ++allierte;
+        }
     }
 
-    for (sf = allies; sf; sf = sf->next) {
-        int mode = alliedgroup(NULL, f, sf->faction, sf, HELP_ALL);
-        if (mode <= 0)
-            continue;
-        i++;
-        if (dh) {
-            if (i == allierte) {
-                bytes = (int)str_strlcpy(bufp, LOC(f->locale, "list_and"), size);
+    if (allierte > 0) {
+        sbstring sbs;
+        sbs_init(&sbs, buf, size);
+
+        for (sf = allies; sf; sf = sf->next) {
+            int mode = alliedgroup(NULL, f, sf->faction, sf, HELP_ALL);
+            if (mode <= 0)
+                continue;
+            i++;
+            if (dh) {
+                if (i == allierte) {
+                    sbs_strcat(&sbs, LOC(f->locale, "list_and"));
+                }
+                else {
+                    sbs_strcat(&sbs, ", ");
+                }
+            }
+            dh = 1;
+            hh = 0;
+            sbs_strcat(&sbs, factionname(sf->faction));
+            sbs_strcat(&sbs, " (");
+            if ((mode & HELP_ALL) == HELP_ALL) {
+                sbs_strcat(&sbs, LOC(f->locale, parameters[P_ANY]));
             }
             else {
-                bytes = (int)str_strlcpy(bufp, ", ", size);
-            }
-            if (wrptr(&bufp, &size, bytes) != 0)
-                WARN_STATIC_BUFFER();
-        }
-        dh = 1;
-        hh = 0;
-        bytes = (int)str_strlcpy(bufp, factionname(sf->faction), size);
-        if (wrptr(&bufp, &size, bytes) != 0)
-            WARN_STATIC_BUFFER();
-        bytes = (int)str_strlcpy(bufp, " (", size);
-        if (wrptr(&bufp, &size, bytes) != 0)
-            WARN_STATIC_BUFFER();
-        if ((mode & HELP_ALL) == HELP_ALL) {
-            bytes = (int)str_strlcpy(bufp, LOC(f->locale, parameters[P_ANY]), size);
-            if (wrptr(&bufp, &size, bytes) != 0)
-                WARN_STATIC_BUFFER();
-        }
-        else {
-            for (h = 1; h <= HELP_TRAVEL; h *= 2) {
-                int p = MAXPARAMS;
-                if ((mode & h) == h) {
-                    switch (h) {
-                    case HELP_TRAVEL:
-                        p = P_TRAVEL;
-                        break;
-                    case HELP_MONEY:
-                        p = P_MONEY;
-                        break;
-                    case HELP_FIGHT:
-                        p = P_FIGHT;
-                        break;
-                    case HELP_GIVE:
-                        p = P_GIVE;
-                        break;
-                    case HELP_GUARD:
-                        p = P_GUARD;
-                        break;
-                    case HELP_FSTEALTH:
-                        p = P_FACTIONSTEALTH;
-                        break;
+                for (h = 1; h <= HELP_TRAVEL; h *= 2) {
+                    int p = MAXPARAMS;
+                    if ((mode & h) == h) {
+                        switch (h) {
+                        case HELP_TRAVEL:
+                            p = P_TRAVEL;
+                            break;
+                        case HELP_MONEY:
+                            p = P_MONEY;
+                            break;
+                        case HELP_FIGHT:
+                            p = P_FIGHT;
+                            break;
+                        case HELP_GIVE:
+                            p = P_GIVE;
+                            break;
+                        case HELP_GUARD:
+                            p = P_GUARD;
+                            break;
+                        case HELP_FSTEALTH:
+                            p = P_FACTIONSTEALTH;
+                            break;
+                        }
+                    }
+                    if (p != MAXPARAMS) {
+                        if (hh) {
+                            sbs_strcat(&sbs, ", ");
+                        }
+                        sbs_strcat(&sbs, LOC(f->locale, parameters[p]));
+                        hh = 1;
                     }
                 }
-                if (p != MAXPARAMS) {
-                    if (hh) {
-                        bytes = (int)str_strlcpy(bufp, ", ", size);
-                        if (wrptr(&bufp, &size, bytes) != 0)
-                            WARN_STATIC_BUFFER();
-                    }
-                    bytes = (int)str_strlcpy(bufp, LOC(f->locale, parameters[p]), size);
-                    if (wrptr(&bufp, &size, bytes) != 0)
-                        WARN_STATIC_BUFFER();
-                    hh = 1;
-                }
             }
+            sbs_strcat(&sbs, ")");
         }
-        bytes = (int)str_strlcpy(bufp, ")", size);
-        if (wrptr(&bufp, &size, bytes) != 0)
-            WARN_STATIC_BUFFER();
+        sbs_strcat(&sbs, ".");
     }
-    bytes = (int)str_strlcpy(bufp, ".", size);
-    if (wrptr(&bufp, &size, bytes) != 0)
-        WARN_STATIC_BUFFER();
-    *bufp = 0;
 }
 
 static void allies(struct stream *out, const faction * f)

From 557c624200891ed63ace706118b8398211162a72 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Fri, 26 Oct 2018 16:08:46 +0200
Subject: [PATCH 10/19] contant name conflict resolved

---
 src/kernel/race.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/kernel/race.c b/src/kernel/race.c
index 31d437992..8a8311991 100644
--- a/src/kernel/race.c
+++ b/src/kernel/race.c
@@ -79,10 +79,10 @@ const char *racenames[MAXRACES] = {
     "clone"
 };
 
-#define MAXOPTIONS 4
+#define MAX_OPTIONS 4
 typedef struct rcoption {
-    unsigned char key[MAXOPTIONS];
-    variant value[MAXOPTIONS];
+    unsigned char key[MAX_OPTIONS];
+    variant value[MAX_OPTIONS];
 } rcoption;
 
 enum {
@@ -106,7 +106,7 @@ static void rc_setoption(race *rc, int k, const char *value) {
         v = rc->options->value;
     } else {
         int i;
-        for (i=0;!v && i < MAXOPTIONS;++i) {
+        for (i=0;!v && i < MAX_OPTIONS;++i) {
             if (rc->options->key[i]==key) {
                 v = rc->options->value+i;
                 break;
@@ -114,7 +114,7 @@ static void rc_setoption(race *rc, int k, const char *value) {
             if (rc->options->key[i]==RCO_NONE) {
                 v = rc->options->value+i;
                 rc->options->key[i] = key;
-                if (i+1 < MAXOPTIONS) {
+                if (i+1 < MAX_OPTIONS) {
                     rc->options->key[i+1]=RCO_NONE;
                 }
                 break;
@@ -145,7 +145,7 @@ static void rc_setoption(race *rc, int k, const char *value) {
 static variant *rc_getoption(const race *rc, int key) {
     if (rc->options) {
         int i;
-        for (i=0;i!=MAXOPTIONS && rc->options->key[i]!=RCO_NONE;++i) {
+        for (i=0;i!=MAX_OPTIONS && rc->options->key[i]!=RCO_NONE;++i) {
             if (rc->options->key[i]==key) {
                 return rc->options->value+i;
             }
@@ -269,7 +269,7 @@ void free_races(void) {
         rcoption * opt = races->options;
         
         if (opt) {
-            for (i=0;i!=MAXOPTIONS && opt->key[i]!=RCO_NONE;++i) {
+            for (i=0;i!=MAX_OPTIONS && opt->key[i]!=RCO_NONE;++i) {
                 if (opt->key[i]==RCO_HUNGER) {
                     free(opt->value[i].v);
                 }

From ac3a4d91c9b9430efcfa2380aa43bec249c7cb61 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Fri, 26 Oct 2018 19:47:50 +0200
Subject: [PATCH 11/19] simplicate the use of ally.c

---
 src/battle.c              | 46 ++++++++------------
 src/creport.c             | 13 +++---
 src/creport.test.c        |  4 +-
 src/give.test.c           |  3 +-
 src/kernel/ally.c         | 92 +++++++++++++++++++++------------------
 src/kernel/ally.h         | 16 ++++---
 src/kernel/ally.test.c    | 25 +++--------
 src/kernel/faction.c      | 19 ++------
 src/kernel/faction.h      |  2 +
 src/kernel/faction.test.c |  8 ++--
 src/kernel/group.c        | 22 ++--------
 src/kernel/group.test.c   |  9 ++--
 src/kernel/plane.h        | 10 ++---
 src/kernel/pool.test.c    | 10 ++---
 src/kernel/save.c         | 16 +++----
 src/kernel/save.test.c    |  5 +--
 src/laws.c                |  8 ++--
 src/laws.test.c           | 12 +++--
 src/move.test.c           |  4 +-
 src/report.c              |  9 ++--
 src/reports.c             |  2 +-
 src/reports.test.c        |  4 +-
 src/study.test.c          |  4 +-
 23 files changed, 140 insertions(+), 203 deletions(-)

diff --git a/src/battle.c b/src/battle.c
index 8b71f17e1..8ed79a732 100644
--- a/src/battle.c
+++ b/src/battle.c
@@ -281,19 +281,15 @@ static void set_friendly(side * as, side * ds)
     as->relations[ds->index] |= E_FRIEND;
 }
 
-static int allysfm(const side * s, const faction * f, int mode)
+static bool alliedside(const side * s, const faction * f, int mode)
 {
-    if (s->faction == f)
-        return mode;
-    if (s->group) {
-        return alliedgroup(s->battle->plane, s->faction, f, s->group->allies, mode);
+    if (s->faction == f) {
+        return true;
     }
-    return alliedfaction(s->battle->plane, s->faction, f, mode);
-}
-
-static int allysf(const side * s, const faction * f)
-{
-    return allysfm(s, f, HELP_FIGHT);
+    if (s->group) {
+        return alliedgroup(s->faction, f, s->group, mode) != 0;
+    }
+    return alliedfaction(s->faction, f, mode) != 0;
 }
 
 static int dead_fighters(const fighter * df)
@@ -313,7 +309,7 @@ fighter *select_corpse(battle * b, fighter * af)
 
     for (si = 0; si != b->nsides; ++si) {
         side *s = b->sides + si;
-        if (af == NULL || (!enemy_i(af->side, si) && allysf(af->side, s->faction))) {
+        if (af == NULL || (!enemy_i(af->side, si) && alliedside(af->side, s->faction, HELP_FIGHT))) {
             maxcasualties += s->casualties;
         }
     }
@@ -344,7 +340,7 @@ bool helping(const side * as, const side * ds)
 {
     if (as->faction == ds->faction)
         return true;
-    return (bool)(!enemy(as, ds) && allysf(as, ds->faction));
+    return (bool)(!enemy(as, ds) && alliedside(as, ds->faction, HELP_FIGHT));
 }
 
 int statusrow(int status)
@@ -832,7 +828,7 @@ bool meffect_protection(battle * b, meffect * s, side * ds)
         return false;
     if (enemy(s->magician->side, ds))
         return false;
-    if (allysf(s->magician->side, ds->faction))
+    if (alliedside(s->magician->side, ds->faction, HELP_FIGHT))
         return true;
     return false;
 }
@@ -1654,7 +1650,7 @@ selist *select_fighters(battle * b, const side * vs, int mask, select_fun cb, vo
                 continue;
         }
         else if (mask == FS_HELP) {
-            if (enemy(s, vs) || !allysf(s, vs->faction)) {
+            if (enemy(s, vs) || !alliedside(s, vs->faction, HELP_FIGHT)) {
                 continue;
             }
         }
@@ -3636,7 +3632,7 @@ static void join_allies(battle * b)
                      * vorgespiegelt wird, und er sich uns gegen�ber nicht zu
                      * erkennen gibt, helfen wir ihm nicht */
                     if (s->stealthfaction) {
-                        if (!allysfm(s, u->faction, HELP_FSTEALTH)) {
+                        if (!alliedside(s, u->faction, HELP_FSTEALTH)) {
                             continue;
                         }
                     }
@@ -3690,17 +3686,13 @@ static void join_allies(battle * b)
         }
 
         for (sa = s + 1; sa != b->sides + b->nsides; ++sa) {
-            plane *pl = rplane(r);
-            if (enemy(s, sa))
-                continue;
-            if (friendly(s, sa))
-                continue;
-            if (!alliedgroup(pl, f, sa->faction, f->allies, HELP_FIGHT))
-                continue;
-            if (!alliedgroup(pl, sa->faction, f, sa->faction->allies, HELP_FIGHT))
-                continue;
-
-            set_friendly(s, sa);
+            if (!enemy(s, sa) && !friendly(s, sa)) {
+                if (alliedfaction(f, sa->faction, HELP_FIGHT)) {
+                    if (alliedfaction(sa->faction, f, HELP_FIGHT)) {
+                        set_friendly(s, sa);
+                    }
+                }
+            }
         }
     }
 }
diff --git a/src/creport.c b/src/creport.c
index 0173b7409..7da7b0810 100644
--- a/src/creport.c
+++ b/src/creport.c
@@ -1021,15 +1021,16 @@ static void cr_output_unit_compat(FILE * F, const faction * f,
 /* = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = =  */
 
 /* prints allies */
-static void show_allies_cr(FILE * F, const faction * f, const ally * sf)
+static void show_allies_cr(FILE * F, const faction * f, const group *g)
 {
+    ally * sf = g ? g->allies : f->allies;
     for (; sf; sf = sf->next)
-        if (sf->faction) {
-            int mode = alliedgroup(NULL, f, sf->faction, sf, HELP_ALL);
+        if (sf->faction && faction_alive(sf->faction)) {
+            int mode = alliedgroup(f, sf->faction, g, HELP_ALL);
             if (mode != 0 && sf->status > 0) {
                 fprintf(F, "ALLIANZ %d\n", sf->faction->no);
                 fprintf(F, "\"%s\";Parteiname\n", sf->faction->name);
-                fprintf(F, "%d;Status\n", sf->status & HELP_ALL);
+                fprintf(F, "%d;Status\n", sf->status);
             }
         }
 }
@@ -1646,7 +1647,7 @@ report_computer(const char *filename, report_context * ctx, const char *bom)
             f->options &= (~flag);
         }
     }
-    show_allies_cr(F, f, f->allies);
+    show_allies_cr(F, f, NULL);
     {
         group *g;
         for (g = f->groups; g; g = g->next) {
@@ -1659,7 +1660,7 @@ report_computer(const char *filename, report_context * ctx, const char *bom)
                 fprintf(F, "\"%s\";typprefix\n",
                     translate(prefix, LOC(f->locale, prefix)));
             }
-            show_allies_cr(F, f, g->allies);
+            show_allies_cr(F, f, g);
         }
     }
 
diff --git a/src/creport.test.c b/src/creport.test.c
index 6bd5bfba8..b536a7114 100644
--- a/src/creport.test.c
+++ b/src/creport.test.c
@@ -227,7 +227,6 @@ static void test_cr_factionstealth(CuTest *tc) {
     faction *f1, *f2;
     region *r;
     unit *u;
-    ally *al;
 
     test_setup();
     f1 = test_create_faction(NULL);
@@ -298,8 +297,7 @@ static void test_cr_factionstealth(CuTest *tc) {
     mstream_done(&strm);
 
     /* we see the same thing as them when we are an ally */
-    al = ally_add(&f1->allies, f2);
-    al->status = HELP_FSTEALTH;
+    ally_set(&f1->allies, f2, HELP_FSTEALTH);
     mstream_init(&strm);
     cr_output_unit(&strm, f2, u, seen_unit);
     CuAssertIntEquals(tc, f1->no, cr_get_int(&strm, ";Partei", -1));
diff --git a/src/give.test.c b/src/give.test.c
index 806901c46..2828456ae 100644
--- a/src/give.test.c
+++ b/src/give.test.c
@@ -44,8 +44,7 @@ static void setup_give(struct give *env) {
     env->itype = it_get_or_create(rt_get_or_create("money"));
     env->itype->flags |= ITF_HERB;
     if (env->f2) {
-        ally * al = ally_add(&env->f2->allies, env->f1);
-        al->status = HELP_GIVE;
+        ally_set(&env->f2->allies, env->f1, HELP_GIVE);
         env->dst = test_create_unit(env->f2, env->r);
     }
     else {
diff --git a/src/kernel/ally.c b/src/kernel/ally.c
index 0660f148a..3c5c5d851 100644
--- a/src/kernel/ally.c
+++ b/src/kernel/ally.c
@@ -8,7 +8,6 @@
 #include "group.h"
 #include "faction.h"
 #include "objtypes.h"
-#include "plane.h"
 
 #include <kernel/attrib.h>
 #include <util/strings.h>
@@ -20,9 +19,8 @@
 #include <string.h>
 #include <stdlib.h>
 
-void read_allies(gamedata * data, faction *f)
+void read_allies(gamedata * data, ally **sfp)
 {
-    ally **sfp = &f->allies;
     for (;;) {
         int aid;
         READ_INT(data->store, &aid);
@@ -62,19 +60,6 @@ ally * ally_add(ally **al_p, struct faction *f) {
     return al;
 }
 
-void ally_remove(ally **al_p, struct faction *f) {
-    ally * al;
-    while (*al_p) {
-        al = *al_p;
-        if (al->faction == f) {
-            *al_p = al->next;
-            free(al);
-            break;
-        }
-        al_p = &al->next;
-    }
-}
-
 static int ally_flag(const char *s, int help_mask)
 {
     if ((help_mask & HELP_MONEY) && strcmp(s, "money") == 0)
@@ -114,11 +99,8 @@ int AllianceAuto(void)
 }
 
 static int
-autoalliance(const plane * pl, const faction * sf, const faction * f2)
+autoalliance(const faction * sf, const faction * f2)
 {
-    if (pl && (pl->flags & PFL_FRIENDLY))
-        return HELP_ALL;
-
     if (f_get_alliance(sf) != NULL && AllianceAuto()) {
         if (sf->alliance == f2->alliance)
             return AllianceAuto();
@@ -195,9 +177,11 @@ static int AllianceRestricted(void)
 }
 
 int
-alliedgroup(const struct plane *pl, const struct faction *f,
-    const struct faction *f2, const struct ally *sf, int mode)
+alliedgroup(const struct faction *f,
+    const struct faction *f2, const struct group *g, int mode)
 {
+    ally *sf = g ? g->allies : f->allies;
+
     if (!(faction_alive(f) && faction_alive(f2))) {
         return 0;
     }
@@ -205,9 +189,9 @@ alliedgroup(const struct plane *pl, const struct faction *f,
         sf = sf->next;
     }
     if (sf == NULL) {
-        mode = mode & autoalliance(pl, f, f2);
+        mode = mode & autoalliance(f, f2);
     }
-    mode = ally_mode(sf, mode) | (mode & autoalliance(pl, f, f2));
+    mode = ally_mode(sf, mode) | (mode & autoalliance(f, f2));
     if (AllianceRestricted()) {
         if (a_find(f->attribs, &at_npcfaction)) {
             return mode;
@@ -223,26 +207,24 @@ alliedgroup(const struct plane *pl, const struct faction *f,
 }
 
 int
-alliedfaction(const struct plane *pl, const struct faction *f,
-    const struct faction *f2, int mode)
+alliedfaction(const struct faction *f, const struct faction *f2, int mode)
 {
-    return alliedgroup(pl, f, f2, f->allies, mode);
+    return alliedgroup(f, f2, NULL, mode);
 }
 
 /* Die Gruppe von Einheit u hat helfe zu f2 gesetzt. */
 int alliedunit(const unit * u, const faction * f2, int mode)
 {
-    int automode;
-
     assert(u);
     assert(f2);
     assert(u->region);            /* the unit should be in a region, but it's possible that u->number==0 (TEMP units) */
-    if (u->faction == f2)
+    if (u->faction == f2) {
         return mode;
+    }
+    if (!faction_alive(f2)) {
+        return 0;
+    }
     if (u->faction != NULL && f2 != NULL) {
-        ally *sf;
-        plane *pl;
-
         if (mode & HELP_FIGHT) {
             if ((u->flags & UFL_DEFENDER) || (u->faction->flags & FFL_DEFENDER)) {
                 faction *owner = region_get_owner(u->region);
@@ -253,20 +235,44 @@ int alliedunit(const unit * u, const faction * f2, int mode)
             }
         }
 
-        pl = rplane(u->region);
-        automode = mode & autoalliance(pl, u->faction, f2);
-
-        if (pl != NULL && (pl->flags & PFL_NOALLIANCES))
-            mode = (mode & automode) | (mode & HELP_GIVE);
-
-        sf = u->faction->allies;
         if (fval(u, UFL_GROUP)) {
             const attrib *a = a_find(u->attribs, &at_group);
-            if (a != NULL)
-                sf = ((group *)a->data.v)->allies;
+            if (a != NULL) {
+                group *g = (group *)a->data.v;
+                return alliedgroup(u->faction, f2, g, mode);
+            }
         }
-        return alliedgroup(pl, u->faction, f2, sf, mode);
+        return alliedfaction(u->faction, f2, mode);
     }
     return 0;
 }
 
+void ally_set(ally **allies, struct faction *f, int status) {
+    ally *al;
+    while (*allies) {
+        al = *allies;
+        if (al->faction == f) {
+            if (status != 0) {
+                al->status = status;
+            }
+            else {
+                *allies = al->next;
+                free(al);
+            }
+            return;
+        }
+        allies = &al->next;
+    }
+    al = ally_add(allies, f);
+    al->status = status;
+}
+
+int ally_get(ally *allies, const struct faction *f) {
+    ally *al;
+    for (al = allies; al; al = al->next) {
+        if (al->faction == f) {
+            return al->status;
+        }
+    }
+    return 0;
+}
diff --git a/src/kernel/ally.h b/src/kernel/ally.h
index d4c41d36c..661edc5a5 100644
--- a/src/kernel/ally.h
+++ b/src/kernel/ally.h
@@ -24,8 +24,8 @@ extern "C" {
 #endif
 
     struct attrib_type;
-    struct plane;
     struct faction;
+    struct group;
     struct gamedata;
     struct unit;
 
@@ -37,19 +37,21 @@ extern "C" {
         int status;
     } ally;
 
-    void read_allies(struct gamedata * data, struct faction *f);
+    int ally_get(struct ally *allies, const struct faction *f);
+    void ally_set(struct ally **allies, struct faction *f, int status);
+
+    void read_allies(struct gamedata * data, struct ally **allies);
     ally * ally_find(ally *al, const struct faction *f);
     ally * ally_add(ally **al_p, struct faction *f);
-    void ally_remove(ally **al_p, struct faction *f);
 
     int AllianceAuto(void);        /* flags that allied factions get automatically */
     int HelpMask(void);    /* flags restricted to allied factions */
     int alliedunit(const struct unit *u, const struct faction *f2,
         int mode);
-    int alliedfaction(const struct plane *pl, const struct faction *f,
-        const struct faction *f2, int mode);
-    int alliedgroup(const struct plane *pl, const struct faction *f,
-        const struct faction *f2, const struct ally *sf, int mode);
+    int alliedfaction(const struct faction *f, const struct faction *f2,
+        int mode);
+    int alliedgroup(const struct faction *f, const struct faction *f2,
+        const struct group *g, int mode);
 
 #ifdef __cplusplus
 }
diff --git a/src/kernel/ally.test.c b/src/kernel/ally.test.c
index 99d220151..3197132a0 100644
--- a/src/kernel/ally.test.c
+++ b/src/kernel/ally.test.c
@@ -7,37 +7,22 @@
 
 static void test_ally(CuTest * tc)
 {
-    ally * al = 0;
+    ally * al = NULL;
     struct faction * f1 = test_create_faction(NULL);
 
-    ally_add(&al, f1);
+    ally_set(&al, f1, HELP_GUARD);
     CuAssertPtrNotNull(tc, al);
-    CuAssertPtrEquals(tc, f1, ally_find(al, f1)->faction);
+    CuAssertIntEquals(tc, HELP_GUARD, ally_get(al, f1));
 
-    ally_remove(&al, f1);
+    ally_set(&al, f1, 0);
     CuAssertPtrEquals(tc, NULL, al);
-    CuAssertPtrEquals(tc, NULL, ally_find(al, f1));
-}
-
-static void test_ally_null(CuTest * tc)
-{
-    ally *a1 = 0, *a2 = 0;
-
-    a1 = ally_add(&a1, 0);
-    a2 = ally_add(&a1, 0);
-    CuAssertPtrNotNull(tc, a1);
-    CuAssertPtrNotNull(tc, a2);
-    CuAssertPtrEquals(tc, a2, a1->next);
-    CuAssertPtrEquals(tc, NULL, a2->next);
-    free(a1);
-    free(a2);
+    CuAssertIntEquals(tc, 0, ally_get(al, f1));
 }
 
 CuSuite *get_ally_suite(void)
 {
     CuSuite *suite = CuSuiteNew();
     SUITE_ADD_TEST(suite, test_ally);
-    SUITE_ADD_TEST(suite, test_ally_null);
     return suite;
 }
 
diff --git a/src/kernel/faction.c b/src/kernel/faction.c
index bb5a27271..d2871f84b 100755
--- a/src/kernel/faction.c
+++ b/src/kernel/faction.c
@@ -496,7 +496,7 @@ void destroyfaction(faction ** fp)
 
     handle_event(f->attribs, "destroy", f);
     if (f->alliance) {
-        setalliance(f, 0);
+        setalliance(f, NULL);
     }
 
     funhash(f);
@@ -531,20 +531,9 @@ int get_alliance(const faction * a, const faction * b)
 
 void set_alliance(faction * a, faction * b, int status)
 {
-    ally **sfp;
-    sfp = &a->allies;
-    while (*sfp) {
-        ally *sf = *sfp;
-        if (sf->faction == b)
-            break;
-        sfp = &sf->next;
-    }
-    if (*sfp == NULL) {
-        ally *sf = ally_add(sfp, b);
-        sf->status = status;
-        return;
-    }
-    (*sfp)->status |= status;
+    /* TODO: optimization (use allies_walk?) */
+    int original = ally_get(a->allies, b);
+    ally_set(&a->allies, b, status | original);
 }
 
 void renumber_faction(faction * f, int no)
diff --git a/src/kernel/faction.h b/src/kernel/faction.h
index c10e6ec69..87b13f2b9 100644
--- a/src/kernel/faction.h
+++ b/src/kernel/faction.h
@@ -111,6 +111,8 @@ extern "C" {
     void fhash(struct faction *f);
     void funhash(struct faction *f);
 
+    int faction_ally_status(const faction *f, const faction *f2);
+
     struct faction *findfaction(int n);
     int max_magicians(const faction * f);
     void set_show_item(faction * f, const struct item_type *itype);
diff --git a/src/kernel/faction.test.c b/src/kernel/faction.test.c
index 9aac0465c..66e3c3945 100644
--- a/src/kernel/faction.test.c
+++ b/src/kernel/faction.test.c
@@ -29,20 +29,18 @@
 static void test_destroyfaction_allies(CuTest *tc) {
     faction *f1, *f2;
     region *r;
-    ally *al;
 
     test_setup();
     r = test_create_region(0, 0, NULL);
     f1 = test_create_faction(NULL);
     test_create_unit(f1, r);
     f2 = test_create_faction(NULL);
-    al = ally_add(&f1->allies, f2);
-    al->status = HELP_FIGHT;
-    CuAssertIntEquals(tc, HELP_FIGHT, alliedgroup(0, f1, f2, f1->allies, HELP_ALL));
+    ally_set(&f1->allies, f2, HELP_FIGHT);
+    CuAssertIntEquals(tc, HELP_FIGHT, alliedfaction(f1, f2, HELP_ALL));
     CuAssertPtrEquals(tc, f2, f1->next);
     destroyfaction(&f1->next);
     CuAssertIntEquals(tc, false, faction_alive(f2));
-    CuAssertIntEquals(tc, 0, alliedgroup(0, f1, f2, f1->allies, HELP_ALL));
+    CuAssertIntEquals(tc, 0, alliedfaction(f1, f2, HELP_ALL));
     test_teardown();
 }
 
diff --git a/src/kernel/group.c b/src/kernel/group.c
index 54aa87aeb..36598d582 100755
--- a/src/kernel/group.c
+++ b/src/kernel/group.c
@@ -72,12 +72,11 @@ static void init_group(faction * f, group * g)
     ally *a, **an;
 
     an = &g->allies;
-    for (a = f->allies; a; a = a->next)
+    for (a = f->allies; a; a = a->next) {
         if (a->faction) {
-            ally *ga = ally_add(an, a->faction);
-            ga->status = a->status;
-            an = &ga->next;
+            ally_set(an, a->faction, a->status);
         }
+    }
 }
 
 static group *find_groupbyname(group * g, const char *name)
@@ -228,7 +227,6 @@ void read_groups(gamedata *data, faction * f)
 {
     struct storage *store = data->store;
     for (;;) {
-        ally **pa;
         group *g;
         int gid;
         char buf[1024];
@@ -238,19 +236,7 @@ void read_groups(gamedata *data, faction * f)
             break;
         READ_STR(store, buf, sizeof(buf));
         g = new_group(f, buf, gid);
-        pa = &g->allies;
-        for (;;) {
-            ally *al;
-            int id;
-            READ_INT(store, &id);
-            if (id == 0) break;
-            al = ally_add(pa, NULL);
-            al->faction = findfaction(id);
-            if (!al->faction) {
-                ur_add(RESOLVE_FACTION | id, (void **)&al->faction, NULL);
-            }
-            READ_INT(store, &al->status);
-        }
+        read_allies(data, &g->allies);
         read_attribs(data, &g->attribs, g);
     }
 }
diff --git a/src/kernel/group.test.c b/src/kernel/group.test.c
index 877728780..c554c0829 100644
--- a/src/kernel/group.test.c
+++ b/src/kernel/group.test.c
@@ -28,7 +28,6 @@ static void test_group_readwrite_dead_faction(CuTest *tc) {
     faction *f, *f2;
     unit * u;
     group *g;
-    ally *al;
     int fno;
 
     test_setup();
@@ -42,8 +41,8 @@ static void test_group_readwrite_dead_faction(CuTest *tc) {
     CuAssertPtrNotNull(tc, u);
     g = join_group(u, "group");
     CuAssertPtrNotNull(tc, g);
-    al = ally_add(&g->allies, f);
-    CuAssertPtrNotNull(tc, al);
+    ally_set(&g->allies, f, HELP_GIVE);
+    CuAssertPtrNotNull(tc, g->allies);
 
     CuAssertPtrEquals(tc, f, factions);
     destroyfaction(&factions);
@@ -73,7 +72,6 @@ static void test_group_readwrite(CuTest * tc)
 {
     faction * f;
     group *g;
-    ally *al;
     int i;
     gamedata data;
     storage store;
@@ -85,8 +83,7 @@ static void test_group_readwrite(CuTest * tc)
     new_group(f, "NW", 42);
     g = new_group(f, "Egoisten", 43);
     key_set(&g->attribs, 44, 44);
-    al = ally_add(&g->allies, f);
-    al->status = HELP_GIVE;
+    ally_set(&g->allies, f, HELP_GIVE);
     write_groups(&store, f);
     WRITE_INT(&store, 47);
 
diff --git a/src/kernel/plane.h b/src/kernel/plane.h
index 5b1c71644..faa663a36 100644
--- a/src/kernel/plane.h
+++ b/src/kernel/plane.h
@@ -28,10 +28,10 @@ extern "C" {
     struct plane;
     struct storage;
 
-#define PFL_NOCOORDS        1   /* not in use */
-#define PFL_NORECRUITS      2
-#define PFL_NOALLIANCES     4
-#define PFL_LOWSTEALING     8
+#define PFL_NOCOORDS        1   /* not implemented */
+#define PFL_NORECRUITS      2   /* cannot recruit */
+#define PFL_NOALLIANCES     4   /* not implemented */
+#define PFL_LOWSTEALING     8   /* not implemented */
 #define PFL_NOGIVE         16   /* Übergaben sind unmöglich */
 #define PFL_NOATTACK       32   /* Angriffe und Diebstähle sind unmöglich */
 #define PFL_NOTERRAIN      64   /* Terraintyp wird nicht angezeigt TODO? */
@@ -40,7 +40,7 @@ extern "C" {
 #define PFL_NOTEACH       512   /* Lehre außer Betrieb */
 #define PFL_NOBUILD      1024   /* Bauen außer Betrieb */
 #define PFL_NOFEED       2048   /* Kein Unterhalt nötig */
-#define PFL_FRIENDLY     4096   /* everyone is your ally */
+#define PFL_FRIENDLY     4096   /* not implemented */
 #define PFL_NOORCGROWTH  8192   /* orcs don't grow */
 #define PFL_NOMONSTERS  16384   /* no monster randenc */
 
diff --git a/src/kernel/pool.test.c b/src/kernel/pool.test.c
index a3de6c021..10c127f81 100644
--- a/src/kernel/pool.test.c
+++ b/src/kernel/pool.test.c
@@ -47,7 +47,6 @@ void test_pool(CuTest *tc) {
     faction *f;
     region *r;
     struct resource_type *rtype;
-    ally *al;
 
     test_setup();
     test_create_world();
@@ -74,10 +73,9 @@ void test_pool(CuTest *tc) {
     CuAssertIntEquals(tc, 100, get_pooled(u1, rtype, GET_POOLED_SLACK, INT_MAX));
     CuAssertIntEquals(tc, 200, get_pooled(u1, rtype, GET_POOLED_SLACK | GET_POOLED_RESERVE, INT_MAX));
 
-    al = ally_add(&u3->faction->allies, f);
-    al->status = HELP_GUARD;
+    ally_set(&u3->faction->allies, f, HELP_GUARD);
     CuAssertIntEquals(tc, 0, get_pooled(u1, rtype, GET_ALLIED_SLACK | GET_ALLIED_RESERVE, INT_MAX));
-    al->status = HELP_MONEY;
+    ally_set(&u3->faction->allies, f, HELP_MONEY);
     CuAssertIntEquals(tc, 200, get_pooled(u1, rtype, GET_ALLIED_SLACK, INT_MAX));
     CuAssertIntEquals(tc, 200, get_pooled(u1, rtype, GET_ALLIED_RESERVE, INT_MAX));
     CuAssertIntEquals(tc, 400, get_pooled(u1, rtype, GET_ALLIED_SLACK | GET_ALLIED_RESERVE, INT_MAX));
@@ -117,7 +115,6 @@ void test_pool_use(CuTest *tc) {
     faction *f;
     region *r;
     struct item_type *itype;
-    ally *al;
 
     test_setup();
     test_create_world();
@@ -135,8 +132,7 @@ void test_pool_use(CuTest *tc) {
     set_resvalue(u2, itype, 100);
     i_change(&u3->items, itype, 400);
     set_resvalue(u3, itype, 200);
-    al = ally_add(&u3->faction->allies, f);
-    al->status = HELP_MONEY;
+    ally_set(&u3->faction->allies, f, HELP_MONEY);
 
     CuAssertIntEquals(tc, 10, use_pooled(u1, itype->rtype, GET_SLACK, 10));
     CuAssertIntEquals(tc, 40, use_pooled(u1, itype->rtype, GET_SLACK, 50));
diff --git a/src/kernel/save.c b/src/kernel/save.c
index b9f501a4e..72ccc66e3 100644
--- a/src/kernel/save.c
+++ b/src/kernel/save.c
@@ -1082,7 +1082,7 @@ faction *read_faction(gamedata * data)
         /* mistakes were made in the past*/
         f->options &= ~WANT_OPTION(O_JSON);
     }
-    read_allies(data, f);
+    read_allies(data, &f->allies);
     read_groups(data, f);
     f->spellbook = 0;
     if (data->version >= REGIONOWNER_VERSION) {
@@ -1145,17 +1145,13 @@ void write_faction(gamedata *data, const faction * f)
     WRITE_SECTION(data->store);
 
     for (sf = f->allies; sf; sf = sf->next) {
-        int no;
-        int status;
-
         assert(sf->faction);
 
-        no = sf->faction->no;
-        status = alliedfaction(NULL, f, sf->faction, HELP_ALL);
-
-        if (status != 0) {
-            WRITE_INT(data->store, no);
-            WRITE_INT(data->store, sf->status);
+        if (faction_alive(sf->faction)) {
+            if (sf->status != 0) {
+                WRITE_INT(data->store, sf->faction->no);
+                WRITE_INT(data->store, sf->status);
+            }
         }
     }
     WRITE_INT(data->store, 0);
diff --git a/src/kernel/save.test.c b/src/kernel/save.test.c
index a70a27c2f..7d51c70ce 100644
--- a/src/kernel/save.test.c
+++ b/src/kernel/save.test.c
@@ -250,7 +250,6 @@ static void test_readwrite_dead_faction_group(CuTest *tc) {
     faction *f, *f2;
     unit * u;
     group *g;
-    ally *al;
     int fno;
     gamedata data;
     storage store;
@@ -269,8 +268,8 @@ static void test_readwrite_dead_faction_group(CuTest *tc) {
     CuAssertPtrNotNull(tc, u);
     g = join_group(u, "group");
     CuAssertPtrNotNull(tc, g);
-    al = ally_add(&g->allies, f);
-    CuAssertPtrNotNull(tc, al);
+    ally_set(&g->allies, f, HELP_GIVE);
+    CuAssertPtrNotNull(tc, g->allies);
 
     CuAssertPtrEquals(tc, f, factions);
     destroyfaction(&factions);
diff --git a/src/laws.c b/src/laws.c
index 6cbe7e726..ba4ad246c 100644
--- a/src/laws.c
+++ b/src/laws.c
@@ -800,7 +800,7 @@ void immigration(void)
 void nmr_warnings(void)
 {
     faction *f, *fa;
-#define FRIEND (HELP_GUARD|HELP_MONEY)
+#define HELP_NMR (HELP_GUARD|HELP_MONEY)
     for (f = factions; f; f = f->next) {
         if (!fval(f, FFL_NOIDLEOUT) && turn > f->lastorders) {
             ADDMSG(&f->msgs, msg_message("nmr_warning", ""));
@@ -816,14 +816,12 @@ void nmr_warnings(void)
                             warn = 1;
                         }
                     }
-                    else if (alliedfaction(NULL, f, fa, FRIEND)
-                        && alliedfaction(NULL, fa, f, FRIEND)) {
+                    else if (alliedfaction(f, fa, HELP_NMR) && alliedfaction(fa, f, HELP_NMR)) {
                         warn = 1;
                     }
                     if (warn) {
                         if (msg == NULL) {
-                            msg =
-                                msg_message("warn_dropout", "faction turns", f,
+                            msg = msg_message("warn_dropout", "faction turns", f,
                                     turn - f->lastorders);
                         }
                         add_message(&fa->msgs, msg);
diff --git a/src/laws.test.c b/src/laws.test.c
index 524c750d4..7531b11f8 100644
--- a/src/laws.test.c
+++ b/src/laws.test.c
@@ -222,7 +222,6 @@ static void test_rule_force_leave(CuTest *tc) {
 }
 
 static void test_force_leave_buildings(CuTest *tc) {
-    ally *al;
     region *r;
     unit *u1, *u2, *u3;
     building * b;
@@ -245,8 +244,7 @@ static void test_force_leave_buildings(CuTest *tc) {
     CuAssertPtrNotNull(tc, test_find_messagetype(u3->faction->msgs, "force_leave_building"));
 
     u_set_building(u3, b);
-    al = ally_add(&u1->faction->allies, u3->faction);
-    al->status = HELP_GUARD;
+    ally_set(&u1->faction->allies, u3->faction, HELP_GUARD);
     force_leave(r, NULL);
     CuAssertPtrEquals_Msg(tc, "allies should not be forced to leave", b, u3->building);
     test_teardown();
@@ -1265,25 +1263,25 @@ static void test_ally_cmd(CuTest *tc) {
     ord = create_order(K_ALLY, f->locale, "%s", itoa36(f->no));
     ally_cmd(u, ord);
     CuAssertPtrEquals(tc, NULL, u->faction->msgs);
-    CuAssertIntEquals(tc, HELP_ALL, alliedfaction(0, u->faction, f, HELP_ALL));
+    CuAssertIntEquals(tc, HELP_ALL, ally_get(u->faction->allies, f));
     free_order(ord);
 
     ord = create_order(K_ALLY, f->locale, "%s %s", itoa36(f->no), LOC(f->locale, parameters[P_NOT]));
     ally_cmd(u, ord);
     CuAssertPtrEquals(tc, NULL, u->faction->msgs);
-    CuAssertIntEquals(tc, 0, alliedfaction(0, u->faction, f, HELP_ALL));
+    CuAssertIntEquals(tc, 0, ally_get(u->faction->allies, f));
     free_order(ord);
 
     ord = create_order(K_ALLY, f->locale, "%s %s", itoa36(f->no), LOC(f->locale, parameters[P_GUARD]));
     ally_cmd(u, ord);
     CuAssertPtrEquals(tc, NULL, u->faction->msgs);
-    CuAssertIntEquals(tc, HELP_GUARD, alliedfaction(0, u->faction, f, HELP_ALL));
+    CuAssertIntEquals(tc, HELP_GUARD, ally_get(u->faction->allies, f));
     free_order(ord);
 
     ord = create_order(K_ALLY, f->locale, "%s %s %s", itoa36(f->no), LOC(f->locale, parameters[P_GUARD]), LOC(f->locale, parameters[P_NOT]));
     ally_cmd(u, ord);
     CuAssertPtrEquals(tc, NULL, u->faction->msgs);
-    CuAssertIntEquals(tc, 0, alliedfaction(0, u->faction, f, HELP_ALL));
+    CuAssertIntEquals(tc, 0, ally_get(u->faction->allies, f));
     free_order(ord);
 
     test_teardown();
diff --git a/src/move.test.c b/src/move.test.c
index a4a65fead..2c91165e2 100644
--- a/src/move.test.c
+++ b/src/move.test.c
@@ -153,7 +153,6 @@ static void test_ship_has_harbormaster_same_faction(CuTest * tc) {
 static void test_ship_has_harbormaster_ally(CuTest * tc) {
     unit *u;
     move_fixture mf;
-    ally *al;
 
     test_setup();
     setup_harbor(&mf);
@@ -161,8 +160,7 @@ static void test_ship_has_harbormaster_ally(CuTest * tc) {
     u = test_create_unit(test_create_faction(NULL), mf.r);
     u->building = mf.b;
     building_set_owner(u);
-    al = ally_add(&u->faction->allies, mf.u->faction);
-    al->status = HELP_GUARD;
+    ally_set(&u->faction->allies, mf.u->faction, HELP_GUARD);
 
     CuAssertIntEquals(tc, SA_HARBOUR, check_ship_allowed(mf.sh, mf.r));
     test_teardown();
diff --git a/src/report.c b/src/report.c
index 6d0901484..a897f29ad 100644
--- a/src/report.c
+++ b/src/report.c
@@ -1518,7 +1518,7 @@ show_allies(const faction * f, const ally * allies, char *buf, size_t size)
     const ally *sf;
    
     for (sf = allies; sf; sf = sf->next) {
-        int mode = alliedgroup(NULL, f, sf->faction, sf, HELP_ALL);
+        int mode = alliedfaction(f, sf->faction, HELP_ALL);
         if (mode > 0) {
             ++allierte;
         }
@@ -1529,7 +1529,7 @@ show_allies(const faction * f, const ally * allies, char *buf, size_t size)
         sbs_init(&sbs, buf, size);
 
         for (sf = allies; sf; sf = sf->next) {
-            int mode = alliedgroup(NULL, f, sf->faction, sf, HELP_ALL);
+            int mode = alliedfaction(f, sf->faction, HELP_ALL);
             if (mode <= 0)
                 continue;
             i++;
@@ -1720,9 +1720,10 @@ static void list_address(struct stream *out, const faction * uf, selist * seenfa
                 str ? str : "");
             if (uf == f)
                 label = '*';
-            else if (is_allied(uf, f))
+            else if (is_allied(uf, f)) {
                 label = 'o';
-            else if (alliedfaction(NULL, uf, f, HELP_ALL))
+            }
+            else if (alliedfaction(uf, f, HELP_ALL))
                 label = '+';
             paragraph(out, buf, 4, 0, label);
         }
diff --git a/src/reports.c b/src/reports.c
index 8088af347..375a28884 100644
--- a/src/reports.c
+++ b/src/reports.c
@@ -969,7 +969,7 @@ bufunit(const faction * f, const unit * u, seen_mode mode, char *buf,
 
     dh = 0;
     if (!getarnt) {
-        if (alliedfaction(rplane(u->region), f, fv, HELP_ALL)) {
+        if (alliedfaction(f, fv, HELP_ALL)) {
             dh = 1;
         }
     }
diff --git a/src/reports.test.c b/src/reports.test.c
index e7fcc282b..8bcc457b9 100644
--- a/src/reports.test.c
+++ b/src/reports.test.c
@@ -177,7 +177,6 @@ static void test_bufunit_fstealth(CuTest *tc) {
     faction *f1, *f2;
     region *r;
     unit *u;
-    ally *al;
     char buf[256];
     struct locale *lang;
 
@@ -232,8 +231,7 @@ static void test_bufunit_fstealth(CuTest *tc) {
     u->flags &= ~UFL_ANON_FACTION;
 
     /* we see the same thing as them when we are an ally */
-    al = ally_add(&f1->allies, f2);
-    al->status = HELP_FSTEALTH;
+    ally_set(&f1->allies, f2, HELP_FSTEALTH);
     bufunit(f2, u, seen_unit, buf, sizeof(buf));
     CuAssertStrEquals(tc, "Hodor (1), TWW (2) (UFO (1)), 1 human.", buf);
 
diff --git a/src/study.test.c b/src/study.test.c
index 3a94bf810..420608e5c 100644
--- a/src/study.test.c
+++ b/src/study.test.c
@@ -636,7 +636,6 @@ static void test_teach_many_to_one(CuTest *tc) {
 static void test_teach_message(CuTest *tc) {
     unit *u, *u1, *u2;
     attrib *a;
-    ally *al;
     teaching_info *teach;
 
     test_setup();
@@ -652,8 +651,7 @@ static void test_teach_message(CuTest *tc) {
     set_level(u1, SK_CROSSBOW, TEACHDIFFERENCE);
     u1->thisorder = create_order(K_TEACH, u->faction->locale, itoa36(u->no));
     u2 = test_create_unit(test_create_faction(NULL), u->region);
-    al = ally_add(&u->faction->allies, u2->faction);
-    al->status = HELP_GUARD;
+    ally_set(&u->faction->allies, u2->faction, HELP_GUARD);
     set_level(u2, SK_CROSSBOW, TEACHDIFFERENCE);
     u2->thisorder = create_order(K_TEACH, u->faction->locale, itoa36(u->no));
     CuAssertTrue(tc, !alliedunit(u, u1->faction, HELP_GUARD));

From 113a6e68c721659129d31f4f9c77b3b1d877b028 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Sun, 28 Oct 2018 21:27:24 +0100
Subject: [PATCH 12/19] BUG 2507: LERNE AUTO with invalid skill.

---
 src/automate.c | 20 +++++++++-------
 src/skill.c    |  5 +++-
 src/study.c    | 65 +++++++++++++++++++++++++++++++++-----------------
 src/study.h    |  1 +
 4 files changed, 59 insertions(+), 32 deletions(-)

diff --git a/src/automate.c b/src/automate.c
index 1a553df29..9a3516c44 100644
--- a/src/automate.c
+++ b/src/automate.c
@@ -34,17 +34,19 @@ int autostudy_init(scholar scholars[], int max_scholars, region *r)
     int nscholars = 0;
 
     for (u = r->units; u; u = u->next) {
-        keyword_t kwd = getkeyword(u->thisorder);
+        keyword_t kwd = init_order(u->thisorder, u->faction->locale);
         if (kwd == K_AUTOSTUDY) {
-            if (long_order_allowed(u) && unit_can_study(u)) {
+            if (long_order_allowed(u)) {
                 scholar * st = scholars + nscholars;
-                init_order(u->thisorder, u->faction->locale);
-                st->sk = getskill(u->faction->locale);
-                st->level = effskill_study(u, st->sk);
-                st->learn = 0;
-                st->u = u;
-                if (++nscholars == max_scholars) {
-                    log_fatal("you must increase MAXSCHOLARS");
+                skill_t sk = getskill(u->faction->locale);
+                if (check_student(u, u->thisorder, sk)) {
+                    st->sk = sk;
+                    st->level = effskill_study(u, st->sk);
+                    st->learn = 0;
+                    st->u = u;
+                    if (++nscholars == max_scholars) {
+                        log_fatal("you must increase MAXSCHOLARS");
+                    }
                 }
             }
             else {
diff --git a/src/skill.c b/src/skill.c
index b279ab488..2521befc6 100644
--- a/src/skill.c
+++ b/src/skill.c
@@ -116,7 +116,9 @@ skill_t get_skill(const char *s, const struct locale * lang)
 int skill_cost(skill_t sk) {
     static int config;
     static int costs[MAXSKILLS];
-    int cost;
+    int cost = -1;
+
+    assert(sk >= 0 && sk < MAXSKILLS);
     switch (sk) {
     case SK_SPY:
         cost = 100;
@@ -146,5 +148,6 @@ int skill_cost(skill_t sk) {
 }
 
 bool expensive_skill(skill_t sk) {
+    assert(sk >= 0 && sk < MAXSKILLS);
     return (sk == SK_MAGIC) || skill_cost(sk) > 0;
 }
diff --git a/src/study.c b/src/study.c
index c978e69ad..738c591d7 100644
--- a/src/study.c
+++ b/src/study.c
@@ -523,6 +523,48 @@ static void msg_teachers(struct selist *teachers, struct unit *u, skill_t sk) {
     selist_foreach_ex(teachers, cb_msg_teach, &cbdata);
 }
 
+bool check_student(const struct unit *u, struct order *ord, skill_t sk) {
+    int err = 0;
+
+    if (sk < 0) {
+        err = 77;
+    }
+    /* Hack: Talente mit Malus -99 koennen nicht gelernt werden */
+    else if (u_race(u)->bonus[sk] == -99) {
+        err = 771;
+    }
+    else {
+        static int config;
+        static bool learn_newskills;
+
+        if (config_changed(&config)) {
+            learn_newskills = config_get_int("study.newskills", 1) != 0;
+        }
+        if (!learn_newskills) {
+            skill *sv = unit_skill(u, sk);
+            if (sv == NULL) {
+                /* we can only learn skills we already have */
+                err = 771;
+            }
+        }
+    }
+    if (err) {
+        if (ord) {
+            cmistake(u, ord, err, MSG_EVENT);
+        }
+        return false;
+    }
+
+    if ((u_race(u)->flags & RCF_NOLEARN) || fval(u, UFL_WERE)) {
+        if (ord) {
+            ADDMSG(&u->faction->msgs,
+                msg_feedback(u, ord, "error_race_nolearn", "race", u_race(u)));
+        }
+        return false;
+    }
+    return true;
+}
+
 int study_cmd(unit * u, order * ord)
 {
     region *r = u->region;
@@ -536,7 +578,6 @@ int study_cmd(unit * u, order * ord)
     skill_t sk;
     int maxalchemy = 0;
     int speed_rule = (study_rule_t)config_get_int("study.speedup", 0);
-    bool learn_newskills = config_get_int("study.newskills", 1) != 0;
     static const race *rc_snotling;
     static int rc_cache;
 
@@ -544,32 +585,12 @@ int study_cmd(unit * u, order * ord)
         rc_snotling = get_race(RC_SNOTLING);
     }
 
-    if (!unit_can_study(u)) {
-        ADDMSG(&u->faction->msgs, msg_feedback(u, ord, "error_race_nolearn", "race",
-            u_race(u)));
-        return -1;
-    }
-
     (void)init_order(ord, u->faction->locale);
     sk = getskill(u->faction->locale);
 
-    if (sk < 0) {
-        cmistake(u, ord, 77, MSG_EVENT);
+    if (!check_student(u, ord, sk)) {
         return -1;
     }
-    /* Hack: Talente mit Malus -99 koennen nicht gelernt werden */
-    if (u_race(u)->bonus[sk] == -99) {
-        cmistake(u, ord, 771, MSG_EVENT);
-        return -1;
-    }
-    if (!learn_newskills) {
-        skill *sv = unit_skill(u, sk);
-        if (sv == NULL) {
-            /* we can only learn skills we already have */
-            cmistake(u, ord, 771, MSG_EVENT);
-            return -1;
-        }
-    }
 
     /* snotlings koennen Talente nur bis T8 lernen */
     if (u_race(u) == rc_snotling) {
diff --git a/src/study.h b/src/study.h
index cca0428b7..f85508dec 100644
--- a/src/study.h
+++ b/src/study.h
@@ -47,6 +47,7 @@ extern "C" {
     skill_t getskill(const struct locale *lang);
     bool is_migrant(struct unit *u);
     int study_cost(struct unit *u, skill_t sk);
+    bool check_student(const struct unit *u, struct order *ord, skill_t sk);
 
     typedef void(*learn_fun)(struct unit *u, skill_t sk, int days);
 

From be84c13a72a88fa6d084c25f44a331ef2ca85d0f Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Sun, 28 Oct 2018 21:28:05 +0100
Subject: [PATCH 13/19] remove test_inject_messagetypes(), make that the
 default behavior in tests.

---
 src/alchemy.test.c         |  1 -
 src/automate.test.c        | 10 ++++-
 src/economy.test.c         |  4 +-
 src/give.test.c            | 18 +++++----
 src/items/xerewards.test.c |  2 -
 src/kernel/build.c         | 41 ++++++++++---------
 src/kernel/build.test.c    |  4 +-
 src/kernel/curse.test.c    |  1 -
 src/kernel/messages.c      | 70 +++++++++++++++++++-------------
 src/kernel/messages.test.c | 17 ++++++++
 src/kernel/order.c         |  2 +-
 src/kernel/spell.test.c    |  1 -
 src/kernel/unit.c          |  4 --
 src/kernel/unit.h          |  1 -
 src/laws.test.c            | 83 ++++++++++++++++++++++----------------
 src/monsters.c             |  4 +-
 src/piracy.test.c          |  2 +
 src/renumber.test.c        | 31 ++++++++------
 src/reports.test.c         |  2 -
 src/spells.test.c          |  9 -----
 src/study.test.c           | 51 ++++++++++++-----------
 src/tests.c                |  8 +---
 src/tests.h                |  1 -
 src/util/message.c         | 10 +++++
 src/util/message.h         |  2 +
 25 files changed, 218 insertions(+), 161 deletions(-)

diff --git a/src/alchemy.test.c b/src/alchemy.test.c
index 7510933fe..32024ba1c 100644
--- a/src/alchemy.test.c
+++ b/src/alchemy.test.c
@@ -26,7 +26,6 @@ static void test_herbsearch(CuTest * tc)
     const item_type *itype;
 
     test_setup();
-    test_inject_messagetypes();
     r = test_create_region(0, 0, NULL);
     rc = rc_get_or_create("dragon");
     rc->flags |= RCF_UNARMEDGUARD;
diff --git a/src/automate.test.c b/src/automate.test.c
index a77376668..6c87b8656 100644
--- a/src/automate.test.c
+++ b/src/automate.test.c
@@ -9,17 +9,22 @@
 #include "kernel/region.h"
 #include "kernel/unit.h"
 
+#include "util/message.h"
+
 #include "tests.h"
 
 #include <CuTest.h>
 
 static void test_autostudy_init(CuTest *tc) {
     scholar scholars[4];
-    unit *u1, *u2, *u3;
+    unit *u1, *u2, *u3, *u4;
     faction *f;
     region *r;
 
     test_setup();
+    mt_create_error(77);
+    mt_create_error(771);
+
     r = test_create_plain(0, 0);
     f = test_create_faction(NULL);
     u1 = test_create_unit(f, r);
@@ -30,8 +35,11 @@ static void test_autostudy_init(CuTest *tc) {
     set_level(u2, SK_ENTERTAINMENT, 2);
     u3 = test_create_unit(f, r);
     u3->thisorder = create_order(K_AUTOSTUDY, f->locale, skillnames[SK_PERCEPTION]);
+    u4 = test_create_unit(test_create_faction(NULL), r);
+    u4->thisorder = create_order(K_AUTOSTUDY, f->locale, "Dudelidu");
     scholars[3].u = NULL;
     CuAssertIntEquals(tc, 3, autostudy_init(scholars, 4, r));
+    CuAssertPtrNotNull(tc, test_find_messagetype(u4->faction->msgs, "error77"));
     CuAssertPtrEquals(tc, u2, scholars[0].u);
     CuAssertIntEquals(tc, 2, scholars[0].level);
     CuAssertIntEquals(tc, 0, scholars[0].learn);
diff --git a/src/economy.test.c b/src/economy.test.c
index 90a4bd260..12fff57f8 100644
--- a/src/economy.test.c
+++ b/src/economy.test.c
@@ -145,6 +145,7 @@ static struct unit *create_recruiter(void) {
 
 static void setup_production(void) {
     init_resources();
+    mt_create_feedback("error_cannotmake");
     mt_create_va(mt_new("produce", NULL), "unit:unit", "region:region", "amount:int", "wanted:int", "resource:resource", MT_NEW_END);
     mt_create_va(mt_new("income", NULL), "unit:unit", "region:region", "amount:int", "wanted:int", "mode:int", MT_NEW_END);
     mt_create_va(mt_new("buy", NULL), "unit:unit", "money:int", MT_NEW_END);
@@ -737,6 +738,7 @@ static void test_loot(CuTest *tc) {
 
     test_setup();
     setup_production();
+    mt_create_error(48); /* unit is unarmed */
     it_silver = test_create_silver();
     config_set("rules.enable_loot", "1");
     u = test_create_unit(f = test_create_faction(NULL), test_create_region(0, 0, NULL));
@@ -746,7 +748,7 @@ static void test_loot(CuTest *tc) {
     test_clear_messages(f);
     arm_unit(u);
     produce(u->region);
-    CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "income")); /* unit is unarmed */
+    CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "income"));
     CuAssertIntEquals(tc, 2 * TAXFRACTION, i_get(u->items, it_silver));
     CuAssertIntEquals(tc, UFL_LONGACTION | UFL_NOTMOVING, fval(u, UFL_LONGACTION | UFL_NOTMOVING));
     test_teardown();
diff --git a/src/give.test.c b/src/give.test.c
index 9c07c8cd0..3f6ffa40a 100644
--- a/src/give.test.c
+++ b/src/give.test.c
@@ -65,11 +65,14 @@ static void setup_give(struct give *env) {
     mt_create_va(mt_new("give", NULL), "unit:unit", "target:unit", "resource:resource", "amount:int", MT_NEW_END);
     mt_create_va(mt_new("give_peasants", NULL), "unit:unit", "resource:resource", "amount:int", MT_NEW_END);
     /* error messages: */
+    mt_create_error(129);
+    mt_create_error(96);
+    mt_create_error(10);
+    mt_create_feedback("feedback_give_forbidden");
+    mt_create_feedback("peasants_give_invalid");
     mt_create_va(mt_new("too_many_units_in_faction", NULL), "unit:unit", "region:region", "command:order", "allowed:int", MT_NEW_END);
     mt_create_va(mt_new("too_many_units_in_alliance", NULL), "unit:unit", "region:region", "command:order", "allowed:int", MT_NEW_END);
     mt_create_va(mt_new("feedback_no_contact", NULL), "unit:unit", "region:region", "command:order", "target:unit", MT_NEW_END);
-    mt_create_va(mt_new("feedback_give_forbidden", NULL), "unit:unit", "region:region", "command:order", MT_NEW_END);
-    mt_create_va(mt_new("peasants_give_invalid", NULL), "unit:unit", "region:region", "command:order", MT_NEW_END);
     mt_create_va(mt_new("giverestriction", NULL), "unit:unit", "region:region", "command:order", "turns:int", MT_NEW_END);
     mt_create_va(mt_new("error_unit_size", NULL), "unit:unit", "region:region", "command:order", "maxsize:int", MT_NEW_END);
     mt_create_va(mt_new("nogive_reserved", NULL), "unit:unit", "region:region", "command:order", "resource:resource", "reservation:int", MT_NEW_END);
@@ -91,7 +94,7 @@ static void test_give_unit(CuTest * tc) {
     give_unit(env.src, env.dst, NULL);
     CuAssertPtrEquals(tc, env.f2, env.src->faction);
     CuAssertIntEquals(tc, 1, env.f2->newbies);
-    CuAssertPtrEquals(tc, 0, env.f1->units);
+    CuAssertPtrEquals(tc, NULL, env.f1->units);
     CuAssertPtrNotNull(tc, test_find_messagetype(env.f1->msgs, "give_person"));
     CuAssertPtrNotNull(tc, test_find_messagetype(env.f2->msgs, "receive_person"));
     test_teardown();
@@ -159,11 +162,12 @@ static void test_give_men_magicians(CuTest * tc) {
     message * msg;
 
     test_setup_ex(tc);
+    mt_create_error(158);
     env.f2 = env.f1 = test_create_faction(NULL);
     setup_give(&env);
     set_level(env.src, SK_MAGIC, 1);
     CuAssertPtrNotNull(tc, msg = give_men(1, env.src, env.dst, NULL));
-    CuAssertStrEquals(tc, "error158", (const char *)msg->parameters[3].v);
+    CuAssertStrEquals(tc, "error158", test_get_messagetype(msg));
     CuAssertIntEquals(tc, 1, env.dst->number);
     CuAssertIntEquals(tc, 1, env.src->number);
     msg_release(msg);
@@ -303,7 +307,7 @@ static void test_give_men_requires_contact(CuTest * tc) {
     ord = create_order(K_GIVE, env.f1->locale, "%s ALLES PERSONEN", itoa36(env.dst->no));
     test_clear_messages(env.f1);
     give_cmd(env.src, ord);
-    CuAssertPtrEquals(tc, 0, test_find_messagetype(env.f1->msgs, "give_person"));
+    CuAssertPtrEquals(tc, NULL, test_find_messagetype(env.f1->msgs, "give_person"));
     CuAssertPtrNotNull(tc, test_find_messagetype(env.f1->msgs, "feedback_no_contact"));
 
     msg_release(msg);
@@ -407,7 +411,7 @@ static void test_give_okay(CuTest * tc) {
     setup_give(&env);
 
     config_set("rules.give.flags", "0");
-    CuAssertPtrEquals(tc, 0, check_give(env.src, env.dst, NULL));
+    CuAssertPtrEquals(tc, NULL, check_give(env.src, env.dst, NULL));
     test_teardown();
 }
 
@@ -450,7 +454,7 @@ static void test_give_new_unit(CuTest * tc) {
     setup_give(&env);
     env.dst->number = 0;
     fset(env.dst, UFL_ISNEW);
-    CuAssertPtrEquals(tc, 0, check_give(env.src, env.dst, NULL));
+    CuAssertPtrEquals(tc, NULL, check_give(env.src, env.dst, NULL));
     test_teardown();
 }
 
diff --git a/src/items/xerewards.test.c b/src/items/xerewards.test.c
index 8e30d88eb..dc9aa2621 100644
--- a/src/items/xerewards.test.c
+++ b/src/items/xerewards.test.c
@@ -18,7 +18,6 @@ static void test_manacrystal(CuTest *tc) {
     unit *u;
 
     test_setup();
-    test_inject_messagetypes();
 
     u = test_create_unit(test_create_faction(NULL), test_create_plain(0, 0));
     itype = test_create_itemtype("manacrystal");
@@ -49,7 +48,6 @@ static void test_skillpotion(CuTest *tc) {
     int initialWeeks_Magic = 0;
 
     test_setup();
-    test_inject_messagetypes();
     u = test_create_unit(test_create_faction(NULL), test_create_plain(0, 0));
     itype = test_create_itemtype("skillpotion");
     change_resource(u, itype->rtype, 2);
diff --git a/src/kernel/build.c b/src/kernel/build.c
index 11324f752..1f96fcda9 100644
--- a/src/kernel/build.c
+++ b/src/kernel/build.c
@@ -500,6 +500,28 @@ static int count_materials(unit *u, const construction *type, int n, int complet
     return n;
 }
 
+int build_skill(unit *u, int basesk, int skill_mod) {
+    int effsk, skills;
+    int dm = get_effect(u, oldpotiontype[P_DOMORE]);
+
+    effsk = basesk + skill_mod;
+    assert(effsk >= 0);
+
+    skills = effsk * u->number;
+
+    /* technically, nimblefinge and domore should be in a global set of
+     * "game"-attributes, (as at_skillmod) but for a while, we're leaving
+     * them in here. */
+
+    if (dm != 0) {
+        /* Auswirkung Schaffenstrunk */
+        if (dm > u->number) dm = u->number;
+        change_effect(u, oldpotiontype[P_DOMORE], -dm);
+        skills += dm * effsk;
+    }
+    return skills;
+}
+
 /** Use up resources for building an object.
 * Build up to 'size' points of 'type', where 'completed'
 * of the first object have already been finished. return the
@@ -521,28 +543,11 @@ int build(unit * u, const construction * ctype, int completed, int want, int ski
         return ECOMPLETE;
     }
     if (con->skill != NOSKILL) {
-        int effsk;
-        int dm = get_effect(u, oldpotiontype[P_DOMORE]);
-
         basesk = effskill(u, con->skill, 0);
         if (basesk == 0)
             return ENEEDSKILL;
 
-        effsk = basesk + skill_mod;
-        assert(effsk >= 0);
-
-        skills = effsk * u->number;
-
-        /* technically, nimblefinge and domore should be in a global set of
-         * "game"-attributes, (as at_skillmod) but for a while, we're leaving
-         * them in here. */
-
-        if (dm != 0) {
-            /* Auswirkung Schaffenstrunk */
-            if (dm > u->number) dm = u->number;
-            change_effect(u, oldpotiontype[P_DOMORE], -dm);
-            skills += dm * effsk;
-        }
+        skills = build_skill(u, basesk, skill_mod);
     }
     for (; want > 0 && skills > 0;) {
         int err, n;
diff --git a/src/kernel/build.test.c b/src/kernel/build.test.c
index 9bae742c2..537f9d860 100644
--- a/src/kernel/build.test.c
+++ b/src/kernel/build.test.c
@@ -31,7 +31,6 @@ typedef struct build_fixture {
 
 static unit * setup_build(build_fixture *bf) {
     test_setup();
-    test_inject_messagetypes();
     init_resources();
 
     test_create_itemtype("stone");
@@ -307,7 +306,6 @@ static void test_build_destroy_road_guard(CuTest *tc)
     order *ord;
 
     test_setup();
-    test_inject_messagetypes();
     test_create_region(1, 0, 0);
     r = test_create_region(0, 0, NULL);
     rsetroad(r, D_EAST, 100);
@@ -340,7 +338,6 @@ static void test_build_destroy_road_limit(CuTest *tc)
     order *ord;
 
     test_setup();
-    test_inject_messagetypes();
     test_create_region(1, 0, 0);
     r = test_create_region(0, 0, NULL);
     rsetroad(r, D_EAST, 100);
@@ -365,6 +362,7 @@ static void test_build_destroy_cmd(CuTest *tc) {
     faction *f;
 
     test_setup();
+    mt_create_error(138);
     u = test_create_unit(f = test_create_faction(NULL), test_create_region(0, 0, NULL));
     u->thisorder = create_order(K_DESTROY, f->locale, NULL);
     CuAssertIntEquals(tc, 138, destroy_cmd(u, u->thisorder));
diff --git a/src/kernel/curse.test.c b/src/kernel/curse.test.c
index 9c74544e1..dc3603d50 100644
--- a/src/kernel/curse.test.c
+++ b/src/kernel/curse.test.c
@@ -51,7 +51,6 @@ typedef struct {
 
 static void setup_curse(curse_fixture *fix, const char *name) {
     test_setup();
-    test_inject_messagetypes();
     fix->r = test_create_region(0, 0, NULL);
     fix->u = test_create_unit(test_create_faction(NULL), fix->r);
     fix->c = create_curse(fix->u, &fix->r->attribs, ct_find(name), 1.0, 1, 1.0, 0);
diff --git a/src/kernel/messages.c b/src/kernel/messages.c
index e0513b00d..48ea13f1f 100644
--- a/src/kernel/messages.c
+++ b/src/kernel/messages.c
@@ -67,6 +67,46 @@ variant v)
     }
 }
 
+static int missing_message_mode;
+
+void message_handle_missing(int mode) {
+    missing_message_mode = mode;
+}
+
+static message *missing_feedback(const char *name, const struct unit *u,
+    const struct region *r, struct order *ord)
+{
+    if (missing_message_mode == MESSAGE_MISSING_ERROR) {
+        log_error("trying to create undefined feedback of type \"%s\"\n", name);
+    }
+    else if (missing_message_mode == MESSAGE_MISSING_REPLACE) {
+        if (strcmp(name, "missing_feedback") != 0) {
+            if (!mt_find("missing_feedback")) {
+                mt_create_va(mt_new("missing_feedback", NULL), "unit:unit",
+                    "region:region", "command:order", "name:string", MT_NEW_END);
+            }
+            return msg_message("missing_feedback", "unit region command name", u, r, ord, name);
+        }
+    }
+    return NULL;
+}
+
+static message *missing_message(const char *name) {
+    if (missing_message_mode == MESSAGE_MISSING_ERROR) {
+        log_error("trying to create undefined message of type \"%s\"\n", name);
+    }
+    else if (missing_message_mode == MESSAGE_MISSING_REPLACE) {
+        log_warning("trying to create undefined message of type \"%s\"\n", name);
+        if (strcmp(name, "missing_message") != 0) {
+            if (!mt_find("missing_message")) {
+                mt_create_va(mt_new("missing_message", NULL), "name:string", MT_NEW_END);
+            }
+            return msg_message("missing_message", "name", name);
+        }
+    }
+    return NULL;
+}
+
 struct message *msg_feedback(const struct unit *u, struct order *ord,
     const char *name, const char *sig, ...)
 {
@@ -80,13 +120,7 @@ struct message *msg_feedback(const struct unit *u, struct order *ord,
     }
 
     if (!mtype) {
-        log_warning("trying to create message of unknown type \"%s\"\n", name);
-        if (!mt_find("missing_feedback")) {
-            mt_create_va(mt_new("missing_feedback", NULL), "unit:unit", 
-                "region:region", "command:order", "name:string", MT_NEW_END);
-        }
-        return msg_message("missing_feedback", "name unit region command",
-            name, u, u->region, ord);
+        return missing_feedback(name, u, u->region, ord);
     }
 
     var.v = (void *)u;
@@ -139,28 +173,6 @@ struct message *msg_feedback(const struct unit *u, struct order *ord,
     return msg_create(mtype, args);
 }
 
-static int missing_message_mode;
-
-void message_handle_missing(int mode) {
-    missing_message_mode = mode;
-}
-
-static message *missing_message(const char *name) {
-    if (missing_message_mode == MESSAGE_MISSING_ERROR) {
-        log_error("trying to create undefined message of type \"%s\"\n", name);
-    }
-    else if (missing_message_mode == MESSAGE_MISSING_REPLACE) {
-        log_warning("trying to create undefined message of type \"%s\"\n", name);
-        if (strcmp(name, "missing_message") != 0) {
-            if (!mt_find("missing_message")) {
-                mt_create_va(mt_new("missing_message", NULL), "name:string", MT_NEW_END);
-            }
-            return msg_message("missing_message", "name", name);
-        }
-    }
-    return NULL;
-}
-
 message *msg_message(const char *name, const char *sig, ...)
 /* msg_message("oops_error", "unit region command", u, r, cmd) */
 {
diff --git a/src/kernel/messages.test.c b/src/kernel/messages.test.c
index 3c9c684c6..4ac106ff3 100644
--- a/src/kernel/messages.test.c
+++ b/src/kernel/messages.test.c
@@ -18,6 +18,23 @@ void test_missing_message(CuTest *tc) {
     CuAssertPtrNotNull(tc, msg);
     CuAssertPtrNotNull(tc, msg->type);
     CuAssertStrEquals(tc, msg->type->name, "missing_message");
+    CuAssertStrEquals(tc, "unknown", (const char *)msg->parameters[0].v);
+    msg_release(msg);
+    test_teardown();
+}
+
+void test_missing_feedback(CuTest *tc) {
+    message *msg;
+    struct locale *lang;
+
+    test_setup();
+    message_handle_missing(MESSAGE_MISSING_REPLACE);
+    lang = test_create_locale();
+    msg = msg_error(NULL, NULL, 77);
+    CuAssertPtrNotNull(tc, msg);
+    CuAssertPtrNotNull(tc, msg->type);
+    CuAssertStrEquals(tc, msg->type->name, "missing_feedback");
+    CuAssertStrEquals(tc, "error77", (const char *)msg->parameters[3].v);
     msg_release(msg);
     test_teardown();
 }
diff --git a/src/kernel/order.c b/src/kernel/order.c
index c14a2b30b..8561744dd 100644
--- a/src/kernel/order.c
+++ b/src/kernel/order.c
@@ -342,7 +342,7 @@ order *parse_order(const char *s, const struct locale * lang)
                 sptr = sp;
                 p = parse_token(&sp, token, sizeof(token));
                 sk = get_skill(p, lang);
-                if (!expensive_skill(sk)) {
+                if (sk == NOSKILL || !expensive_skill(sk)) {
                     kwd = K_AUTOSTUDY;
                 }
             }
diff --git a/src/kernel/spell.test.c b/src/kernel/spell.test.c
index 622780e20..025584114 100644
--- a/src/kernel/spell.test.c
+++ b/src/kernel/spell.test.c
@@ -33,7 +33,6 @@ static void test_create_duplicate_spell(CuTest * tc)
     strlist *sl = 0;
 
     test_setup();
-    test_inject_messagetypes();
     test_log_stderr(0); /* suppress the "duplicate spell" error message */
     log = test_log_start(LOG_CPERROR, &sl);
 
diff --git a/src/kernel/unit.c b/src/kernel/unit.c
index 770723bea..b63e8beb8 100644
--- a/src/kernel/unit.c
+++ b/src/kernel/unit.c
@@ -1853,10 +1853,6 @@ bool unit_name_equals_race(const unit *u) {
     return false;
 }
 
-bool unit_can_study(const unit *u) {
-    return !((u_race(u)->flags & RCF_NOLEARN) || fval(u, UFL_WERE));
-}
-
 static int read_newunitid(const faction * f, const region * r)
 {
     int n;
diff --git a/src/kernel/unit.h b/src/kernel/unit.h
index a6135715c..0b0e9da6f 100644
--- a/src/kernel/unit.h
+++ b/src/kernel/unit.h
@@ -246,7 +246,6 @@ extern "C" {
     const char *unitname(const struct unit *u);
     char *write_unitname(const struct unit *u, char *buffer, size_t size);
     bool unit_name_equals_race(const struct unit *u);
-    bool unit_can_study(const struct unit *u);
 
     /* getunit results: */
 #define GET_UNIT 0
diff --git a/src/laws.test.c b/src/laws.test.c
index c7c7a6682..99490c8c3 100644
--- a/src/laws.test.c
+++ b/src/laws.test.c
@@ -153,8 +153,8 @@ static void test_enter_building(CuTest * tc)
     rc->flags = RCF_SWIM;
     u->building = 0;
     CuAssertIntEquals(tc, 0, enter_building(u, NULL, b->no, false));
-    CuAssertPtrEquals(tc, 0, u->building);
-    CuAssertPtrEquals(tc, 0, u->faction->msgs);
+    CuAssertPtrEquals(tc, NULL, u->building);
+    CuAssertPtrEquals(tc, NULL, u->faction->msgs);
 
     CuAssertIntEquals(tc, 0, enter_building(u, NULL, b->no, true));
     CuAssertPtrNotNull(tc, u->faction->msgs);
@@ -170,7 +170,6 @@ static void test_enter_ship(CuTest * tc)
     race * rc;
 
     test_setup();
-
     r = test_create_region(0, 0, NULL);
     rc = test_create_race("smurf");
     u = test_create_unit(test_create_faction(rc), r);
@@ -194,8 +193,8 @@ static void test_enter_ship(CuTest * tc)
     rc->flags = RCF_SWIM;
     u->ship = 0;
     CuAssertIntEquals(tc, 0, enter_ship(u, NULL, sh->no, false));
-    CuAssertPtrEquals(tc, 0, u->ship);
-    CuAssertPtrEquals(tc, 0, u->faction->msgs);
+    CuAssertPtrEquals(tc, NULL, u->ship);
+    CuAssertPtrEquals(tc, NULL, u->faction->msgs);
 
     CuAssertIntEquals(tc, 0, enter_ship(u, NULL, sh->no, true));
     CuAssertPtrNotNull(tc, u->faction->msgs);
@@ -486,6 +485,10 @@ static void test_limit_new_units(CuTest * tc)
     alliance *al;
 
     test_setup();
+    mt_create_va(mt_new("too_many_units_in_faction", NULL), "unit:unit",
+        "region:region", "command:order", "allowed:int", MT_NEW_END);
+    mt_create_va(mt_new("too_many_units_in_alliance", NULL), "unit:unit",
+        "region:region", "command:order", "allowed:int", MT_NEW_END);
     al = makealliance(1, "Hodor");
     f = test_create_faction(NULL);
     u = test_create_unit(f, test_create_region(0, 0, NULL));
@@ -901,6 +904,9 @@ static unit * setup_name_cmd(void) {
     faction *f;
 
     test_setup();
+    mt_create_error(84);
+    mt_create_error(148);
+    mt_create_error(12);
     mt_create_va(mt_new("renamed_building_seen", NULL), "renamer:unit", "region:region", "building:building", MT_NEW_END);
     mt_create_va(mt_new("renamed_building_notseen", NULL), "region:region", "building:building", MT_NEW_END);
     f = test_create_faction(NULL);
@@ -1049,8 +1055,8 @@ static void test_long_order_normal(CuTest *tc) {
     CuAssertIntEquals(tc, 0, fval(u, UFL_MOVED));
     CuAssertIntEquals(tc, 0, fval(u, UFL_LONGACTION));
     CuAssertPtrNotNull(tc, u->orders);
-    CuAssertPtrEquals(tc, 0, u->faction->msgs);
-    CuAssertPtrEquals(tc, 0, u->old_orders);
+    CuAssertPtrEquals(tc, NULL, u->faction->msgs);
+    CuAssertPtrEquals(tc, NULL, u->old_orders);
     test_teardown();
 }
 
@@ -1060,9 +1066,9 @@ static void test_long_order_none(CuTest *tc) {
     test_setup();
     u = test_create_unit(test_create_faction(NULL), test_create_region(0, 0, NULL));
     update_long_order(u);
-    CuAssertPtrEquals(tc, 0, u->thisorder);
-    CuAssertPtrEquals(tc, 0, u->orders);
-    CuAssertPtrEquals(tc, 0, u->faction->msgs);
+    CuAssertPtrEquals(tc, NULL, u->thisorder);
+    CuAssertPtrEquals(tc, NULL, u->orders);
+    CuAssertPtrEquals(tc, NULL, u->faction->msgs);
     test_teardown();
 }
 
@@ -1074,9 +1080,9 @@ static void test_long_order_cast(CuTest *tc) {
     unit_addorder(u, create_order(K_CAST, u->faction->locale, NULL));
     unit_addorder(u, create_order(K_CAST, u->faction->locale, NULL));
     update_long_order(u);
-    CuAssertPtrEquals(tc, 0, u->thisorder);
+    CuAssertPtrEquals(tc, NULL, u->thisorder);
     CuAssertPtrNotNull(tc, u->orders);
-    CuAssertPtrEquals(tc, 0, u->faction->msgs);
+    CuAssertPtrEquals(tc, NULL, u->faction->msgs);
     test_teardown();
 }
 
@@ -1089,9 +1095,9 @@ static void test_long_order_buy_sell(CuTest *tc) {
     unit_addorder(u, create_order(K_SELL, u->faction->locale, NULL));
     unit_addorder(u, create_order(K_SELL, u->faction->locale, NULL));
     update_long_order(u);
-    CuAssertPtrEquals(tc, 0, u->thisorder);
+    CuAssertPtrEquals(tc, NULL, u->thisorder);
     CuAssertPtrNotNull(tc, u->orders);
-    CuAssertPtrEquals(tc, 0, u->faction->msgs);
+    CuAssertPtrEquals(tc, NULL, u->faction->msgs);
     test_teardown();
 }
 
@@ -1099,6 +1105,7 @@ static void test_long_order_multi_long(CuTest *tc) {
     /* TODO: write more tests */
     unit *u;
     test_setup();
+    mt_create_error(52);
     u = test_create_unit(test_create_faction(NULL), test_create_region(0, 0, NULL));
     unit_addorder(u, create_order(K_MOVE, u->faction->locale, NULL));
     unit_addorder(u, create_order(K_DESTROY, u->faction->locale, NULL));
@@ -1113,11 +1120,12 @@ static void test_long_order_multi_buy(CuTest *tc) {
     /* TODO: write more tests */
     unit *u;
     test_setup();
+    mt_create_error(52);
     u = test_create_unit(test_create_faction(NULL), test_create_region(0, 0, NULL));
     unit_addorder(u, create_order(K_BUY, u->faction->locale, 0));
     unit_addorder(u, create_order(K_BUY, u->faction->locale, 0));
     update_long_order(u);
-    CuAssertPtrEquals(tc, 0, u->thisorder);
+    CuAssertPtrEquals(tc, NULL, u->thisorder);
     CuAssertPtrNotNull(tc, u->orders);
     CuAssertPtrNotNull(tc, test_find_messagetype(u->faction->msgs, "error52"));
     test_teardown();
@@ -1132,9 +1140,9 @@ static void test_long_order_multi_sell(CuTest *tc) {
     unit_addorder(u, create_order(K_BUY, u->faction->locale, 0));
     unit_addorder(u, create_order(K_SELL, u->faction->locale, 0));
     update_long_order(u);
-    CuAssertPtrEquals(tc, 0, u->thisorder);
+    CuAssertPtrEquals(tc, NULL, u->thisorder);
     CuAssertPtrNotNull(tc, u->orders);
-    CuAssertPtrEquals(tc, 0, u->faction->msgs);
+    CuAssertPtrEquals(tc, NULL, u->faction->msgs);
     test_teardown();
 }
 
@@ -1142,11 +1150,12 @@ static void test_long_order_buy_cast(CuTest *tc) {
     /* TODO: write more tests */
     unit *u;
     test_setup();
+    mt_create_error(52);
     u = test_create_unit(test_create_faction(NULL), test_create_region(0, 0, NULL));
     unit_addorder(u, create_order(K_BUY, u->faction->locale, 0));
     unit_addorder(u, create_order(K_CAST, u->faction->locale, 0));
     update_long_order(u);
-    CuAssertPtrEquals(tc, 0, u->thisorder);
+    CuAssertPtrEquals(tc, NULL, u->thisorder);
     CuAssertPtrNotNull(tc, u->orders);
     CuAssertPtrNotNull(tc, test_find_messagetype(u->faction->msgs, "error52"));
     test_teardown();
@@ -1164,7 +1173,7 @@ static void test_long_order_hungry(CuTest *tc) {
     update_long_order(u);
     CuAssertIntEquals(tc, K_WORK, getkeyword(u->thisorder));
     CuAssertPtrNotNull(tc, u->orders);
-    CuAssertPtrEquals(tc, 0, u->faction->msgs);
+    CuAssertPtrEquals(tc, NULL, u->faction->msgs);
     test_teardown();
 }
 
@@ -1174,9 +1183,10 @@ static void test_ally_cmd_errors(CuTest *tc) {
     order *ord;
 
     test_setup();
+    mt_create_error(66);
     u = test_create_unit(test_create_faction(NULL), test_create_region(0, 0, NULL));
     fid = u->faction->no + 1;
-    CuAssertPtrEquals(tc, 0, findfaction(fid));
+    CuAssertPtrEquals(tc, NULL, findfaction(fid));
 
     ord = create_order(K_ALLY, u->faction->locale, itoa36(fid));
     ally_cmd(u, ord);
@@ -1273,25 +1283,25 @@ static void test_ally_cmd(CuTest *tc) {
 
     ord = create_order(K_ALLY, f->locale, "%s", itoa36(f->no));
     ally_cmd(u, ord);
-    CuAssertPtrEquals(tc, 0, u->faction->msgs);
+    CuAssertPtrEquals(tc, NULL, u->faction->msgs);
     CuAssertIntEquals(tc, HELP_ALL, alliedfaction(0, u->faction, f, HELP_ALL));
     free_order(ord);
 
     ord = create_order(K_ALLY, f->locale, "%s %s", itoa36(f->no), LOC(f->locale, parameters[P_NOT]));
     ally_cmd(u, ord);
-    CuAssertPtrEquals(tc, 0, u->faction->msgs);
+    CuAssertPtrEquals(tc, NULL, u->faction->msgs);
     CuAssertIntEquals(tc, 0, alliedfaction(0, u->faction, f, HELP_ALL));
     free_order(ord);
 
     ord = create_order(K_ALLY, f->locale, "%s %s", itoa36(f->no), LOC(f->locale, parameters[P_GUARD]));
     ally_cmd(u, ord);
-    CuAssertPtrEquals(tc, 0, u->faction->msgs);
+    CuAssertPtrEquals(tc, NULL, u->faction->msgs);
     CuAssertIntEquals(tc, HELP_GUARD, alliedfaction(0, u->faction, f, HELP_ALL));
     free_order(ord);
 
     ord = create_order(K_ALLY, f->locale, "%s %s %s", itoa36(f->no), LOC(f->locale, parameters[P_GUARD]), LOC(f->locale, parameters[P_NOT]));
     ally_cmd(u, ord);
-    CuAssertPtrEquals(tc, 0, u->faction->msgs);
+    CuAssertPtrEquals(tc, NULL, u->faction->msgs);
     CuAssertIntEquals(tc, 0, alliedfaction(0, u->faction, f, HELP_ALL));
     free_order(ord);
 
@@ -1325,6 +1335,8 @@ static unit * setup_mail_cmd(void) {
     faction *f;
     
     test_setup();
+    mt_create_error(66);
+    mt_create_error(30);
     mt_create_va(mt_new("regionmessage", NULL), "region:region", "sender:unit", "string:string", MT_NEW_END);
     mt_create_va(mt_new("unitmessage", NULL), "region:region", "sender:unit", "string:string", "unit:unit", MT_NEW_END);
     mt_create_va(mt_new("mail_result", NULL), "message:string", "unit:unit", MT_NEW_END);
@@ -1383,7 +1395,7 @@ static void test_mail_unit_no_msg(CuTest *tc) {
     f = u->faction;
     ord = create_order(K_MAIL, f->locale, "%s %s", LOC(f->locale, parameters[P_UNIT]), itoa36(u->no));
     mail_cmd(u, ord);
-    CuAssertPtrEquals(tc, 0, test_find_messagetype(f->msgs, "unitmessage"));
+    CuAssertPtrEquals(tc, NULL, test_find_messagetype(f->msgs, "unitmessage"));
     CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "error30"));
     free_order(ord);
     test_teardown();
@@ -1398,7 +1410,7 @@ static void test_mail_faction_no_msg(CuTest *tc) {
     f = u->faction;
     ord = create_order(K_MAIL, f->locale, "%s %s", LOC(f->locale, parameters[P_FACTION]), itoa36(f->no));
     mail_cmd(u, ord);
-    CuAssertPtrEquals(tc, 0, test_find_messagetype(f->msgs, "regionmessage"));
+    CuAssertPtrEquals(tc, NULL, test_find_messagetype(f->msgs, "regionmessage"));
     CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "error30"));
     free_order(ord);
     test_teardown();
@@ -1413,7 +1425,7 @@ static void test_mail_faction_no_target(CuTest *tc) {
     f = u->faction;
     ord = create_order(K_MAIL, f->locale, "%s %s", LOC(f->locale, parameters[P_FACTION]), itoa36(f->no+1));
     mail_cmd(u, ord);
-    CuAssertPtrEquals(tc, 0, test_find_messagetype(f->msgs, "regionmessage"));
+    CuAssertPtrEquals(tc, NULL, test_find_messagetype(f->msgs, "regionmessage"));
     CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "error66"));
     free_order(ord);
     test_teardown();
@@ -1428,7 +1440,7 @@ static void test_mail_region_no_msg(CuTest *tc) {
     f = u->faction;
     ord = create_order(K_MAIL, f->locale, LOC(f->locale, parameters[P_REGION]));
     mail_cmd(u, ord);
-    CuAssertPtrEquals(tc, 0, test_find_messagetype(u->region->msgs, "mail_result"));
+    CuAssertPtrEquals(tc, NULL, test_find_messagetype(u->region->msgs, "mail_result"));
     CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "error30"));
     free_order(ord);
     test_teardown();
@@ -1444,6 +1456,8 @@ static void test_show_without_item(CuTest *tc)
     struct locale *loc;
 
     test_setup();
+    mt_create_error(21);
+    mt_create_error(36);
     mt_create_va(mt_new("displayitem", NULL), "weight:int", "item:resource", "description:string", MT_NEW_END);
 
     loc = get_or_create_locale("de");
@@ -1466,14 +1480,14 @@ static void test_show_without_item(CuTest *tc)
     locale_setstring(loc, "iteminfo::testitem", "testdescription");
 
     reshow_cmd(u, ord);
-    CuAssertPtrEquals(tc, 0, test_find_messagetype(f->msgs, "error21"));
+    CuAssertPtrEquals(tc, NULL, test_find_messagetype(f->msgs, "error21"));
     CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "error36"));
     test_clear_messages(f);
 
     i_add(&(u->items), i_new(itype, 1));
     reshow_cmd(u, ord);
-    CuAssertPtrEquals(tc, 0, test_find_messagetype(f->msgs, "error21"));
-    CuAssertPtrEquals(tc, 0, test_find_messagetype(f->msgs, "error36"));
+    CuAssertPtrEquals(tc, NULL, test_find_messagetype(f->msgs, "error21"));
+    CuAssertPtrEquals(tc, NULL, test_find_messagetype(f->msgs, "error36"));
     test_clear_messages(f);
 
     free_order(ord);
@@ -1488,7 +1502,6 @@ static void test_show_race(CuTest *tc) {
     message * msg;
 
     test_setup();
-
     mt_create_va(mt_new("msg_event", NULL), "string:string", MT_NEW_END);
     test_create_race("human");
     rc = test_create_race("elf");
@@ -1504,8 +1517,8 @@ static void test_show_race(CuTest *tc) {
 
     ord = create_order(K_RESHOW, loc, "Mensch");
     reshow_cmd(u, ord);
-    CuAssertTrue(tc, test_find_messagetype(u->faction->msgs, "error21") != NULL);
-    CuAssertTrue(tc, test_find_messagetype(u->faction->msgs, "msg_event") == NULL);
+    CuAssertPtrNotNull(tc, test_find_messagetype(u->faction->msgs, "error21"));
+    CuAssertPtrEquals(tc, NULL, test_find_messagetype(u->faction->msgs, "msg_event"));
     test_clear_messages(u->faction);
     free_order(ord);
 
@@ -1618,7 +1631,7 @@ static void test_demon_hunger(CuTest * tc)
     get_food(r);
 
     CuAssertIntEquals(tc, 20, i_get(u->items, rtype->itype));
-    CuAssertPtrEquals(tc, 0, test_find_messagetype(f->msgs, "malnourish"));
+    CuAssertPtrEquals(tc, NULL, test_find_messagetype(f->msgs, "malnourish"));
 
     config_set("hunger.demon.peasant_tolerance", "0");
 
diff --git a/src/monsters.c b/src/monsters.c
index 89a678f11..cd59f9b23 100644
--- a/src/monsters.c
+++ b/src/monsters.c
@@ -555,7 +555,7 @@ static order *monster_learn(unit * u)
     const struct locale *lang = u->faction->locale;
 
     /* can these monsters even study? */
-    if (!unit_can_study(u)) {
+    if (!check_student(u, NULL, SK_PERCEPTION)) {
         return NULL;
     }
 
@@ -828,7 +828,7 @@ void plan_monsters(faction * f)
                     }
                 }
             }
-            if (long_order == NULL && unit_can_study(u)) {
+            if (long_order == NULL && check_student(u, NULL, SK_WEAPONLESS)) {
                 /* Einheiten, die Waffenlosen Kampf lernen k�nnten, lernen es um
                 * zu bewachen: */
                 if (rc->bonus[SK_WEAPONLESS] != -99) {
diff --git a/src/piracy.test.c b/src/piracy.test.c
index 096e267b7..9d2603d42 100644
--- a/src/piracy.test.c
+++ b/src/piracy.test.c
@@ -30,6 +30,8 @@ static void setup_piracy(void) {
     st_boat = test_create_shiptype("boat");
     st_boat->cargo = 1000;
 
+    mt_create_error(144);
+    mt_create_error(146);
     mt_create_va(mt_new("piratenovictim", NULL),
         "ship:ship", "unit:unit", "region:region", MT_NEW_END);
     mt_create_va(mt_new("piratesawvictim", NULL),
diff --git a/src/renumber.test.c b/src/renumber.test.c
index 98ca9b48b..8ad0c6e12 100644
--- a/src/renumber.test.c
+++ b/src/renumber.test.c
@@ -14,12 +14,19 @@
 #include <stddef.h>
 #include <CuTest.h>
 
+static void setup_renumber(CuTest *tc) {
+    test_setup_ex(tc);
+    mt_create_error(114);
+    mt_create_error(115);
+    mt_create_error(116);
+}
+
 static void test_renumber_faction(CuTest *tc) {
     unit *u;
     int uno, no;
     const struct locale *lang;
 
-    test_setup_ex(tc);
+    setup_renumber(tc);
     u = test_create_unit(test_create_faction(NULL), test_create_region(0, 0, NULL));
     no = u->faction->no;
     uno = (no > 1) ? no - 1 : no + 1;
@@ -37,7 +44,7 @@ static void test_renumber_faction_duplicate(CuTest *tc) {
     int no;
     const struct locale *lang;
 
-    test_setup_ex(tc);
+    setup_renumber(tc);
     mt_create_va(mt_new("renumber_inuse", NULL), "id:int", MT_NEW_END);
     f2 = test_create_faction(NULL);
     u = test_create_unit(f = test_create_faction(NULL), test_create_region(0, 0, NULL));
@@ -57,7 +64,7 @@ static void test_renumber_faction_invalid(CuTest *tc) {
     int no;
     const struct locale *lang;
 
-    test_setup_ex(tc);
+    setup_renumber(tc);
     u = test_create_unit(f = test_create_faction(0), test_create_region(0, 0, 0));
     no = f->no;
     lang = f->locale;
@@ -87,7 +94,7 @@ static void test_renumber_building(CuTest *tc) {
     int uno, no;
     const struct locale *lang;
 
-    test_setup_ex(tc);
+    setup_renumber(tc);
     u = test_create_unit(test_create_faction(NULL), test_create_region(0, 0, NULL));
     u->building = test_create_building(u->region, NULL);
     no = u->building->no;
@@ -105,7 +112,7 @@ static void test_renumber_building_duplicate(CuTest *tc) {
     int uno, no;
     const struct locale *lang;
 
-    test_setup_ex(tc);
+    setup_renumber(tc);
     u = test_create_unit(f = test_create_faction(NULL), test_create_region(0, 0, NULL));
     u->building = test_create_building(u->region, NULL);
     uno = u->building->no;
@@ -124,7 +131,7 @@ static void test_renumber_ship(CuTest *tc) {
     int uno, no;
     const struct locale *lang;
 
-    test_setup_ex(tc);
+    setup_renumber(tc);
     u = test_create_unit(test_create_faction(NULL), test_create_region(0, 0, NULL));
     u->ship = test_create_ship(u->region, NULL);
     no = u->ship->no;
@@ -141,7 +148,7 @@ static void test_renumber_ship_twice(CuTest *tc) {
     int uno, no;
     const struct locale *lang;
 
-    test_setup_ex(tc);
+    setup_renumber(tc);
     u = test_create_unit(test_create_faction(NULL), test_create_region(0, 0, NULL));
     u->ship = test_create_ship(u->region, NULL);
     no = u->ship->no;
@@ -163,7 +170,7 @@ static void test_renumber_ship_duplicate(CuTest *tc) {
     int uno, no;
     const struct locale *lang;
 
-    test_setup_ex(tc);
+    setup_renumber(tc);
     u = test_create_unit(f = test_create_faction(NULL), test_create_region(0, 0, NULL));
     u->ship = test_create_ship(u->region, NULL);
     uno = u->ship->no;
@@ -182,7 +189,7 @@ static void test_renumber_unit(CuTest *tc) {
     int uno, no;
     const struct locale *lang;
 
-    test_setup_ex(tc);
+    setup_renumber(tc);
     u = test_create_unit(test_create_faction(NULL), test_create_region(0, 0, NULL));
     no = u->no;
     uno = (no > 1) ? no - 1 : no + 1;
@@ -200,7 +207,7 @@ static void test_renumber_unit_duplicate(CuTest *tc) {
     int no;
     const struct locale *lang;
 
-    test_setup_ex(tc);
+    setup_renumber(tc);
     u = test_create_unit(f = test_create_faction(NULL), test_create_region(0, 0, NULL));
     no = u->no;
     u2 = test_create_unit(f, u->region);
@@ -219,7 +226,7 @@ static void test_renumber_unit_limit(CuTest *tc) {
     int no;
     const struct locale *lang;
 
-    test_setup_ex(tc);
+    setup_renumber(tc);
     u = test_create_unit(f = test_create_faction(NULL), test_create_region(0, 0, NULL));
     no = u->no;
     lang = f->locale;
@@ -237,7 +244,7 @@ static void test_renumber_unit_invalid(CuTest *tc) {
     int no;
     const struct locale *lang;
 
-    test_setup_ex(tc);
+    setup_renumber(tc);
     u = test_create_unit(f = test_create_faction(NULL), test_create_region(0, 0, NULL));
     no = u->no;
     lang = f->locale;
diff --git a/src/reports.test.c b/src/reports.test.c
index 65b5b06b0..6c19719e2 100644
--- a/src/reports.test.c
+++ b/src/reports.test.c
@@ -791,7 +791,6 @@ static void test_insect_warnings(CuTest *tc) {
 
     test_setup();
     test_create_calendar();
-    test_inject_messagetypes();
     f = test_create_faction(test_create_race("insect"));
 
     CuAssertIntEquals(tc, SEASON_AUTUMN, get_gamedate(1083, &gd)->season);
@@ -819,7 +818,6 @@ static void test_newbie_warning(CuTest *tc) {
     faction *f;
 
     test_setup();
-    test_inject_messagetypes();
     f = test_create_faction(NULL);
     config_set_int("NewbieImmunity", 3);
 
diff --git a/src/spells.test.c b/src/spells.test.c
index e6150cb46..1e024ef7f 100644
--- a/src/spells.test.c
+++ b/src/spells.test.c
@@ -27,10 +27,6 @@
 #include <string.h>
 #include <assert.h>
 
-static void setup_spells(void) {
-    test_inject_messagetypes();
-}
-
 static void test_good_dreams(CuTest *tc) {
     struct region *r;
     struct faction *f1, *f2;
@@ -40,7 +36,6 @@ static void test_good_dreams(CuTest *tc) {
     curse *curse;
     
     test_setup();
-    setup_spells();
     test_create_world();
     r = findregion(0, 0);
     f1 = test_create_faction(NULL);
@@ -70,7 +65,6 @@ static void test_dreams(CuTest *tc) {
     castorder co;
 
     test_setup();
-    setup_spells();
     r = test_create_region(0, 0, NULL);
     f1 = test_create_faction(NULL);
     f2 = test_create_faction(NULL);
@@ -98,7 +92,6 @@ static void test_bad_dreams(CuTest *tc) {
     curse *curse;
     
     test_setup();
-    setup_spells();
     test_create_world();
     r = findregion(0, 0);
     f1 = test_create_faction(NULL);
@@ -129,7 +122,6 @@ static void test_view_reality(CuTest *tc) {
     castorder co;
 
     test_setup();
-    setup_spells();
     mt_create_va(mt_new("spell_astral_only", NULL),
         "unit:unit", "region:region", "command:order", MT_NEW_END);
     mt_create_va(mt_new("viewreality_effect", NULL),
@@ -165,7 +157,6 @@ static void test_watch_region(CuTest *tc) {
     region *r;
     faction *f;
     test_setup();
-    setup_spells();
     r = test_create_region(0, 0, NULL);
     f = test_create_faction(NULL);
     CuAssertIntEquals(tc, -1, get_observer(r, f));
diff --git a/src/study.test.c b/src/study.test.c
index a63be7cf1..679615203 100644
--- a/src/study.test.c
+++ b/src/study.test.c
@@ -58,6 +58,11 @@ typedef struct {
 } study_fixture;
 
 static void setup_study(void) {
+    test_setup();
+    mt_create_error(178);
+    mt_create_error(65);
+    mt_create_va(mt_new("teach_asgood", NULL),
+        "unit:unit", "region:region", "command:order", "student:unit", MT_NEW_END);
     mt_create_va(mt_new("studycost", NULL),
         "unit:unit", "region:region", "cost:int", "skill:int", MT_NEW_END);
     mt_create_va(mt_new("teach_teacher", NULL),
@@ -81,7 +86,7 @@ static void setup_teacher(study_fixture *fix, skill_t sk) {
     struct locale *lang;
 
     assert(fix);
-    test_setup();
+    setup_study();
     config_set("study.random_progress", "0");
     r = test_create_region(0, 0, NULL);
     f = test_create_faction(NULL);
@@ -148,7 +153,7 @@ static void test_study_bug_2194(CuTest *tc) {
     struct locale * loc;
     building * b;
 
-    test_setup();
+    setup_study();
     random_source_inject_constant(0.0);
     init_resources();
     loc = test_create_locale();
@@ -205,7 +210,7 @@ static void test_produceexp(CuTest *tc) {
     unit *u;
 
     g_tc = tc;
-    test_setup();
+    setup_study();
     u = test_create_unit(test_create_faction(NULL), test_create_region(0, 0, NULL));
     scale_number(u, 2);
     config_set("study.produceexp", "20");
@@ -220,7 +225,7 @@ static void test_academy_building(CuTest *tc) {
     building * b;
     message * msg;
 
-    test_setup();
+    setup_study();
     mt_create_va(mt_new("teach_asgood", NULL),
         "unit:unit", "region:region", "command:order", "student:unit", MT_NEW_END);
 
@@ -271,7 +276,6 @@ static void test_academy_bonus(CuTest *tc) {
     struct locale * loc;
     building * b;
 
-    test_setup();
     setup_study();
 
     random_source_inject_constant(0.0);
@@ -328,7 +332,8 @@ static void test_academy_bonus(CuTest *tc) {
 void test_learn_skill_single(CuTest *tc) {
     unit *u;
     skill *sv;
-    test_setup();
+
+    setup_study();
     config_set("study.random_progress", "0");
     u = test_create_unit(test_create_faction(NULL), test_create_region(0, 0, NULL));
     learn_skill(u, SK_ALCHEMY, STUDYDAYS);
@@ -347,7 +352,8 @@ void test_learn_skill_single(CuTest *tc) {
 void test_learn_skill_multi(CuTest *tc) {
     unit *u;
     skill *sv;
-    test_setup();
+
+    setup_study();
     config_set("study.random_progress", "0");
     u = test_create_unit(test_create_faction(NULL), test_create_region(0, 0, NULL));
     scale_number(u, 10);
@@ -367,7 +373,8 @@ void test_learn_skill_multi(CuTest *tc) {
 static void test_demon_skillchanges(CuTest *tc) {
     unit * u;
     const race * rc;
-    test_setup();
+
+    setup_study();
     rc = test_create_race("demon");
     CuAssertPtrEquals(tc, (void *)rc, (void *)get_race(RC_DAEMON));
     u = test_create_unit(test_create_faction(rc), test_create_region(0, 0, NULL));
@@ -380,7 +387,8 @@ static void test_demon_skillchanges(CuTest *tc) {
 
 static void test_study_cmd(CuTest *tc) {
     unit *u;
-    test_setup();
+
+    setup_study();
     init_resources();
     u = test_create_unit(test_create_faction(NULL), test_create_region(0, 0, NULL));
     u->thisorder = create_order(K_STUDY, u->faction->locale, "CROSSBOW");
@@ -399,7 +407,6 @@ static void test_study_magic(CuTest *tc) {
     const struct locale *lang;
     const struct item_type *itype;
 
-    test_setup();
     setup_study();
     init_resources();
     f = test_create_faction(NULL);
@@ -432,7 +439,6 @@ static void test_study_magic(CuTest *tc) {
 static void test_study_cost_magic(CuTest *tc) {
     unit * u;
 
-    test_setup();
     setup_study();
     u = test_create_unit(test_create_faction(NULL), test_create_region(0, 0, NULL));
 
@@ -456,7 +462,6 @@ static void test_study_cost(CuTest *tc) {
     unit *u;
     const struct item_type *itype;
 
-    test_setup();
     setup_study();
 
     itype = test_create_silver();
@@ -484,7 +489,6 @@ static void test_teach_magic(CuTest *tc) {
     faction *f;
     const struct item_type *itype;
 
-    test_setup();
     setup_study();
     init_resources();
     itype = get_resourcetype(R_SILVER)->itype;
@@ -510,7 +514,8 @@ static void test_teach_magic(CuTest *tc) {
 
 static void test_teach_cmd(CuTest *tc) {
     unit *u, *ut;
-    test_setup();
+    
+    setup_study();
     init_resources();
     u = test_create_unit(test_create_faction(NULL), test_create_region(0, 0, NULL));
     scale_number(u, 10);
@@ -530,7 +535,8 @@ static void test_teach_cmd(CuTest *tc) {
 
 static void test_teach_two(CuTest *tc) {
     unit *u1, *u2, *ut;
-    test_setup();
+    
+    setup_study();
     init_resources();
     u1 = test_create_unit(test_create_faction(NULL), test_create_region(0, 0, NULL));
     scale_number(u1, 5);
@@ -560,7 +566,7 @@ static void test_teach_two_skills(CuTest *tc) {
     faction *f;
     region *r;
 
-    test_setup();
+    setup_study();
     init_resources();
     f = test_create_faction(NULL);
     r = test_create_region(0, 0, NULL);
@@ -590,7 +596,8 @@ static void test_teach_two_skills(CuTest *tc) {
 
 static void test_teach_one_to_many(CuTest *tc) {
     unit *u, *ut;
-    test_setup();
+
+    setup_study();
     init_resources();
     u = test_create_unit(test_create_faction(NULL), test_create_region(0, 0, NULL));
     scale_number(u, 20);
@@ -611,7 +618,7 @@ static void test_teach_one_to_many(CuTest *tc) {
 static void test_teach_many_to_one(CuTest *tc) {
     unit *u, *u1, *u2;
 
-    test_setup();
+    setup_study();
     init_resources();
     u = test_create_unit(test_create_faction(NULL), test_create_region(0, 0, NULL));
     scale_number(u, 20);
@@ -639,11 +646,7 @@ static void test_teach_message(CuTest *tc) {
     ally *al;
     teaching_info *teach;
 
-    test_setup();
-    mt_create_va(mt_new("teach_teacher", NULL),
-        "teacher:unit", "student:unit", "skill:int", "level:int", MT_NEW_END);
-    mt_create_va(mt_new("teach_student", NULL),
-        "teacher:unit", "student:unit", "skill:int", MT_NEW_END);
+    setup_study();
     init_resources();
     u = test_create_unit(test_create_faction(NULL), test_create_region(0, 0, NULL));
     scale_number(u, 20);
@@ -683,7 +686,7 @@ static void test_teach_many_to_many(CuTest *tc) {
     region *r;
     faction *f;
 
-    test_setup();
+    setup_study();
     init_resources();
     f = test_create_faction(NULL);
     r = test_create_region(0, 0, NULL);
diff --git a/src/tests.c b/src/tests.c
index f8035e129..6c6ce7b58 100644
--- a/src/tests.c
+++ b/src/tests.c
@@ -277,15 +277,10 @@ void test_create_calendar(void) {
     month_season[8] = SEASON_SUMMER;
 }
 
-void test_inject_messagetypes(void)
-{
-    message_handle_missing(MESSAGE_MISSING_REPLACE);
-}
-
 void test_setup_test(CuTest *tc, const char *file, int line) {
     test_log_stderr(LOG_CPERROR);
     test_reset();
-    message_handle_missing(MESSAGE_MISSING_ERROR);
+    message_handle_missing(MESSAGE_MISSING_REPLACE);
     if (tc) {
         log_debug("start test: %s", tc->name);
     }
@@ -297,6 +292,7 @@ void test_setup_test(CuTest *tc, const char *file, int line) {
 
 void test_teardown(void)
 {
+    message_handle_missing(MESSAGE_MISSING_IGNORE);
     test_reset();
     test_log_stderr(0);
 }
diff --git a/src/tests.h b/src/tests.h
index 441db9e16..d2a77d1b4 100644
--- a/src/tests.h
+++ b/src/tests.h
@@ -75,7 +75,6 @@ extern "C" {
     void assert_string_parameter(struct CuTest * tc, struct message *msg, int index, const char *arg);
 
     void disabled_test(void *suite, void (*)(struct CuTest *), const char *name);
-    void test_inject_messagetypes(void);
 
 #define DISABLE_TEST(SUITE, TEST) disabled_test(SUITE, TEST, #TEST)
 
diff --git a/src/util/message.c b/src/util/message.c
index 8915dca8f..82258e02f 100644
--- a/src/util/message.c
+++ b/src/util/message.c
@@ -198,6 +198,16 @@ message_type *mt_create_va(message_type *mtype, ...)
     return mt_create(mtype, args, i - 1);
 }
 
+message_type *mt_create_feedback(const char *name) {
+    return mt_create_va(mt_new(name, NULL), "unit:unit", "region:region", "command:order", MT_NEW_END);
+}
+
+message_type *mt_create_error(int error) {
+    char name[16];
+    snprintf(name, sizeof(name), "error%d", error);
+    return mt_create_feedback(name);
+}
+
 static variant copy_arg(const arg_type * atype, variant data)
 {
     assert(atype != NULL);
diff --git a/src/util/message.h b/src/util/message.h
index 5aa16232b..52b087203 100644
--- a/src/util/message.h
+++ b/src/util/message.h
@@ -64,6 +64,8 @@ extern "C" {
     struct message_type *mt_new(const char *name, const char *section);
     /** message_type registry (optional): **/
     struct message_type *mt_create(struct message_type *, const char *args[], int nargs);
+    struct message_type *mt_create_feedback(const char *name);
+    struct message_type *mt_create_error(int error);
     struct message_type *mt_create_va(struct message_type *, ...);
     const struct message_type *mt_find(const char *);
 

From dce5f7857be2e9dc7c69ec4a28eb0c2f8a7d42b2 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Sun, 28 Oct 2018 21:31:58 +0100
Subject: [PATCH 14/19] unused variable

---
 src/kernel/messages.test.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/kernel/messages.test.c b/src/kernel/messages.test.c
index 4ac106ff3..9e66638d9 100644
--- a/src/kernel/messages.test.c
+++ b/src/kernel/messages.test.c
@@ -25,11 +25,9 @@ void test_missing_message(CuTest *tc) {
 
 void test_missing_feedback(CuTest *tc) {
     message *msg;
-    struct locale *lang;
 
     test_setup();
     message_handle_missing(MESSAGE_MISSING_REPLACE);
-    lang = test_create_locale();
     msg = msg_error(NULL, NULL, 77);
     CuAssertPtrNotNull(tc, msg);
     CuAssertPtrNotNull(tc, msg->type);

From e5207fd1c7b858b7c551f9bc873a1ad0506ff643 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Sun, 28 Oct 2018 21:55:34 +0100
Subject: [PATCH 15/19] do not backup eressea.db right now

---
 process/backup-eressea | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/process/backup-eressea b/process/backup-eressea
index 4e5f69002..0f81cf53e 100755
--- a/process/backup-eressea
+++ b/process/backup-eressea
@@ -34,4 +34,4 @@ fi
 tar cjf backup/$TURN.tar.bz2 $files
 echo "uploading game-$GAME/$TURN.tar.bz2"
 curl -s -n -T backup/$TURN.tar.bz2 https://dav.box.com/dav/Eressea/game-$GAME/$TURN.tar.bz2
-curl -s -n -T eressea.db https://dav.box.com/dav/Eressea/eressea.db
+#curl -s -n -T eressea.db https://dav.box.com/dav/Eressea/eressea.db

From 89d752bc1ff4ff9447de49ae7813ad75fa408fe1 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Mon, 29 Oct 2018 16:19:29 +0100
Subject: [PATCH 16/19] BUG 2508: wipe edges with 0

---
 src/report.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/report.c b/src/report.c
index abad4c4fb..89d13ca85 100644
--- a/src/report.c
+++ b/src/report.c
@@ -902,6 +902,7 @@ void report_region(struct stream *out, const region * r, faction * f)
     assert(f);
     assert(r);
 
+    memset(edges, 0, sizeof(edges));
     for (d = 0; d != MAXDIRECTIONS; d++) {
         /* Nachbarregionen, die gesehen werden, ermitteln */
         region *r2 = rconnect(r, d);

From c54ab6a3df6bab56a8e0061950032abcc1c204ea Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Mon, 29 Oct 2018 20:06:18 +0100
Subject: [PATCH 17/19] fix CID 189401 and 189402

---
 src/bind_eressea.c     | 6 +++++-
 src/kernel/db/sqlite.c | 7 ++++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/bind_eressea.c b/src/bind_eressea.c
index 4d3918899..f76d0a903 100755
--- a/src/bind_eressea.c
+++ b/src/bind_eressea.c
@@ -36,12 +36,16 @@ int eressea_write_game(const char * filename) {
 
 int eressea_read_orders(const char * filename) {
     FILE * F = fopen(filename, "r");
+    int result;
+
     if (!F) {
         perror(filename);
         return -1;
     }
     log_info("reading orders from %s", filename);
-    return parseorders(F);
+    result = parseorders(F);
+    fclose(F);
+    return result;
 }
 
 int eressea_export_json(const char * filename, int flags) {
diff --git a/src/kernel/db/sqlite.c b/src/kernel/db/sqlite.c
index fd9d0bc67..cc331b551 100644
--- a/src/kernel/db/sqlite.c
+++ b/src/kernel/db/sqlite.c
@@ -9,6 +9,7 @@
 #include <sqlite3.h>
 
 #include <assert.h>
+#include <errno.h>
 #include <limits.h>
 #include <stdlib.h>
 #include <stdio.h>
@@ -213,7 +214,11 @@ void db_driver_close(database_t db)
             FILE * F = fopen(g_swapname, "r");
             if (F) {
                 fclose(F);
-                remove(g_swapname);
+                if (0 != remove(g_swapname)) {
+                    log_error("could not remove %s: %s", g_swapname,
+                            strerror(errno));
+                    errno = 0;
+                }
             }
         }
     }

From 253849416aec7fbb2102befc30eca56bb4df7d66 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Tue, 30 Oct 2018 06:30:32 +0100
Subject: [PATCH 18/19] create referenced factions early instead of
 read_faction_reference.

---
 src/kernel/ally.c    | 2 +-
 src/kernel/faction.c | 8 ++++++++
 src/kernel/faction.h | 1 +
 src/kernel/save.c    | 5 ++---
 4 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/kernel/ally.c b/src/kernel/ally.c
index 3c5c5d851..4b6e29429 100644
--- a/src/kernel/ally.c
+++ b/src/kernel/ally.c
@@ -28,7 +28,7 @@ void read_allies(gamedata * data, ally **sfp)
             ally * al = ally_add(sfp, NULL);
             int state;
             if ((al->faction = findfaction(aid)) == NULL) {
-                ur_add(RESOLVE_FACTION | aid, (void **)&al->faction, NULL);
+                al->faction = faction_create(aid);
             }
             READ_INT(data->store, &state);
             al->status = state & HELP_ALL;
diff --git a/src/kernel/faction.c b/src/kernel/faction.c
index d2871f84b..51833b46f 100755
--- a/src/kernel/faction.c
+++ b/src/kernel/faction.c
@@ -875,3 +875,11 @@ void free_factions(void) {
     free_flist(&factions);
     free_flist(&dead_factions);
 }
+
+faction *faction_create(int no)
+{
+    faction *f = (faction *)calloc(1, sizeof(faction));
+    f->no = no;
+    fhash(f);
+    return f;
+}
diff --git a/src/kernel/faction.h b/src/kernel/faction.h
index 87b13f2b9..f9f7b8cc9 100644
--- a/src/kernel/faction.h
+++ b/src/kernel/faction.h
@@ -127,6 +127,7 @@ extern "C" {
     void destroyfaction(faction ** f);
 
     bool faction_alive(const struct faction *f);
+    struct faction *faction_create(int no);
 
     void set_alliance(struct faction *a, struct faction *b, int status);
     int get_alliance(const struct faction *a, const struct faction *b);
diff --git a/src/kernel/save.c b/src/kernel/save.c
index 72ccc66e3..c8e29f6ed 100644
--- a/src/kernel/save.c
+++ b/src/kernel/save.c
@@ -957,8 +957,7 @@ faction *read_faction(gamedata * data)
     assert(n > 0);
     f = findfaction(n);
     if (f == NULL) {
-        f = (faction *)calloc(1, sizeof(faction));
-        f->no = n;
+        f = faction_create(n);
     }
     else {
         f->allies = NULL;           /* FIXME: mem leak */
@@ -1415,7 +1414,6 @@ int read_game(gamedata *data)
 
         *fp = f;
         fp = &f->next;
-        fhash(f);
     }
     *fp = 0;
 
@@ -1515,6 +1513,7 @@ int read_game(gamedata *data)
             }
         }
         else {
+            assert(f->units);
             for (u = f->units; u; u = u->nextF) {
                 if (data->version < SPELL_LEVEL_VERSION) {
                     sc_mage *mage = get_mage_depr(u);

From bf9c50cfc5ac7cedd5caeefd9a7a42c1498d9ab1 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Mon, 29 Oct 2018 20:12:54 +0100
Subject: [PATCH 19/19] gcc flag to prevent use of VLA

---
 src/CMakeLists.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index c42e3eb2e..96f67ebd0 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -21,11 +21,11 @@ COMPILE_DEFINITIONS ERESSEA_BUILDNO="${ERESSEA_BUILDNO}")
 ENDIF()
 
 IF (CMAKE_COMPILER_IS_GNUCC)
-#    SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-error=unused-but-set-variable")
+    SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wvla")
 ENDIF()
 IF (CMAKE_COMPILER_IS_GNUCC OR CMAKE_C_COMPILER_ID MATCHES "Clang")
 #    SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wconversion -Wno-sign-conversion")
-    SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wsign-compare -Wall -Werror -Wno-unknown-pragmas -Wstrict-prototypes -Wpointer-arith -Wno-char-subscripts -Wno-long-long")
+SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wsign-compare -Wall -Werror -Wno-unknown-pragmas -Wstrict-prototypes -Wpointer-arith -Wno-char-subscripts -Wno-long-long")
 #    SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=c89")
 ELSEIF(MSVC)
     SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /W4 /WX /MP /D_CRT_SECURE_NO_WARNINGS /D_USE_MATH_DEFINES")