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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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 600762809607a9ada74f5916330e7ca4c2744347 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Fri, 26 Oct 2018 14:00:48 +0200
Subject: [PATCH 09/27] report.c is next

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

diff --git a/src/report.c b/src/report.c
index 5d2ed4598..e27044127 100644
--- a/src/report.c
+++ b/src/report.c
@@ -1511,7 +1511,7 @@ report_template(const char *filename, report_context * ctx, const char *bom)
 }
 
 static void
-show_allies(const faction * f, const ally * allies, char *buf, size_t size)
+show_allies(const faction * f, struct ally * allies, char *buf, size_t size)
 {
     int allierte = 0;
     int i = 0, h, hh = 0;

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 10/27] 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 11/27] 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 12/27] 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 d7a8a9b406381545f9096644252699c6e790fa26 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Fri, 26 Oct 2018 21:49:58 +0200
Subject: [PATCH 13/27] fix that catastrophic merge, make struct ally
 module-private

---
 res/translations/strings.de.po |   2 +-
 src/bind_faction.c             |  45 ----------
 src/kernel/ally.c              |  51 +++++++++--
 src/kernel/ally.h              |  14 +--
 src/kernel/ally.test.c         |  37 ++++++--
 src/kernel/faction.c           |  18 ----
 src/kernel/faction.h           |   3 -
 src/kernel/group.c             |  29 ++-----
 src/kernel/group.h             |   2 +-
 src/kernel/group.test.c        |   6 +-
 src/kernel/save.c              |  15 +---
 src/kernel/save.test.c         |   1 +
 src/report.c                   | 152 ++++++++++++++++++---------------
 src/upkeep.c                   |   2 +-
 src/upkeep.test.c              |   3 +-
 15 files changed, 180 insertions(+), 200 deletions(-)

diff --git a/res/translations/strings.de.po b/res/translations/strings.de.po
index 47d70ecc0..89875baf5 100644
--- a/res/translations/strings.de.po
+++ b/res/translations/strings.de.po
@@ -1437,7 +1437,7 @@ msgstr "Goblins"
 
 msgctxt "spellinfo"
 msgid "song_of_slavery"
-msgstr "Dieser mächtige Bann raubt dem Opfer seinen freien Willen und unterwirft sie den Befehlen des Barden. Für einige Zeit wird das Opfer sich völlig von seinen eigenen Leuten abwenden und der Partei des Barden zugehörig fühlen."
+msgstr "Dieser mächtige Bann raubt dem Opfer seinen freien Willen und unterwirft es den Befehlen des Barden. Für einige Zeit wird das Opfer sich völlig von seinen eigenen Leuten abwenden und der Partei des Barden zugehörig fühlen."
 
 msgctxt "spell"
 msgid "healingzone"
diff --git a/src/bind_faction.c b/src/bind_faction.c
index 46967252c..fd384ff4f 100644
--- a/src/bind_faction.c
+++ b/src/bind_faction.c
@@ -309,49 +309,6 @@ static int tolua_faction_count_msg_type(lua_State *L) {
     return 1;
 }
 
-static int tolua_faction_get_policy(lua_State * L)
-{
-    faction *self = (faction *)tolua_tousertype(L, 1, 0);
-    faction *other = (faction *)tolua_tousertype(L, 2, 0);
-    const char *policy = tolua_tostring(L, 3, 0);
-
-    int result = 0, mode;
-    for (mode = 0; helpmodes[mode].name != NULL; ++mode) {
-        if (strcmp(policy, helpmodes[mode].name) == 0) {
-            result = get_alliance(self, other) & mode;
-            break;
-        }
-    }
-
-    lua_pushinteger(L, result);
-    return 1;
-}
-
-static int tolua_faction_set_policy(lua_State * L)
-{
-    faction *self = (faction *)tolua_tousertype(L, 1, 0);
-    faction *other = (faction *)tolua_tousertype(L, 2, 0);
-    const char *policy = tolua_tostring(L, 3, 0);
-    int value = tolua_toboolean(L, 4, 0);
-
-    int mode;
-    for (mode = 0; helpmodes[mode].name != NULL; ++mode) {
-        if (strcmp(policy, helpmodes[mode].name) == 0) {
-            if (value) {
-                set_alliance(self, other, get_alliance(self,
-                    other) | helpmodes[mode].status);
-            }
-            else {
-                set_alliance(self, other, get_alliance(self,
-                    other) & ~helpmodes[mode].status);
-            }
-            break;
-        }
-    }
-
-    return 0;
-}
-
 static int tolua_faction_normalize(lua_State * L)
 {
     faction *f = (faction *)tolua_tousertype(L, 1, 0);
@@ -632,8 +589,6 @@ void tolua_faction_open(lua_State * L)
             tolua_variable(L, TOLUA_CAST "lastturn", tolua_faction_get_lastturn,
                 tolua_faction_set_lastturn);
 
-            tolua_function(L, TOLUA_CAST "set_policy", tolua_faction_set_policy);
-            tolua_function(L, TOLUA_CAST "get_policy", tolua_faction_get_policy);
             tolua_function(L, TOLUA_CAST "get_origin", tolua_faction_get_origin);
             tolua_function(L, TOLUA_CAST "set_origin", tolua_faction_set_origin);
             tolua_function(L, TOLUA_CAST "normalize", tolua_faction_normalize);
diff --git a/src/kernel/ally.c b/src/kernel/ally.c
index 20fb364d4..b439975f3 100644
--- a/src/kernel/ally.c
+++ b/src/kernel/ally.c
@@ -25,7 +25,29 @@ typedef struct ally {
     int status;
 } ally;
 
-int allies_walk(struct ally *allies, cb_allies_walk callback, void *udata)
+void allies_free(ally *al)
+{
+    while (al) {
+        ally * an = al->next;
+        free(al);
+        al = an;
+    }
+}
+
+ally *allies_clone(const ally *al) {
+    ally *al_clone = NULL, **al_end = &al_clone;
+
+    for (; al; al = al->next) {
+        if (al->faction) {
+            ally * al_new = ally_add(al_end, al->faction);
+            al_new->status = al->status;
+            al_end = &al_new->next;
+        }
+    }
+    return al_clone;
+}
+
+int allies_walk(ally *allies, cb_allies_walk callback, void *udata)
 {
     ally *al;
     for (al = allies; al; al = al->next) {
@@ -37,6 +59,18 @@ int allies_walk(struct ally *allies, cb_allies_walk callback, void *udata)
     return 0;
 }
 
+void write_allies(gamedata * data, const ally *alist)
+{
+    const ally *a;
+    for (a = alist; a; a = a->next) {
+        if (a->faction && a->faction->_alive) {
+            write_faction_reference(a->faction, data->store);
+            WRITE_INT(data->store, a->status);
+        }
+    }
+    write_faction_reference(NULL, data->store);
+}
+
 void read_allies(gamedata * data, ally **sfp)
 {
     for (;;) {
@@ -196,8 +230,7 @@ static int AllianceRestricted(void)
     return rule;
 }
 
-
-int alliance_status(faction *f, faction *f2, int status) {
+int alliance_status(const faction *f, const faction *f2, int status) {
     status |= autoalliance(f, f2);
     if (status > 0) {
         int mask = AllianceRestricted();
@@ -226,7 +259,7 @@ alliedgroup(const struct faction *f,
     if (!(faction_alive(f) && faction_alive(f2))) {
         return 0;
     }
-    status = ally_get(all, f2);
+    status = ally_get(all, f2) & mask;
     return alliance_status(f, f2, status);
 }
 
@@ -237,19 +270,19 @@ alliedfaction(const struct faction *f, const struct faction *f2, int mask)
 }
 
 /* Die Gruppe von Einheit u hat helfe zu f2 gesetzt. */
-int alliedunit(const unit * u, const faction * f2, int mode)
+int alliedunit(const unit * u, const faction * f2, int mask)
 {
     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) {
-        return mode;
+        return mask;
     }
     if (!faction_alive(f2)) {
         return 0;
     }
     if (u->faction != NULL && f2 != NULL) {
-        if (mode & HELP_FIGHT) {
+        if (mask & HELP_FIGHT) {
             if ((u->flags & UFL_DEFENDER) || (u->faction->flags & FFL_DEFENDER)) {
                 faction *owner = region_get_owner(u->region);
                 /* helps the owner of the region */
@@ -263,10 +296,10 @@ int alliedunit(const unit * u, const faction * f2, int mode)
             const attrib *a = a_find(u->attribs, &at_group);
             if (a != NULL) {
                 group *g = (group *)a->data.v;
-                return alliedgroup(u->faction, f2, g, mode);
+                return alliedgroup(u->faction, f2, g, mask);
             }
         }
-        return alliedfaction(u->faction, f2, mode);
+        return alliedfaction(u->faction, f2, mask);
     }
     return 0;
 }
diff --git a/src/kernel/ally.h b/src/kernel/ally.h
index 70b1a88e0..eeb573cb5 100644
--- a/src/kernel/ally.h
+++ b/src/kernel/ally.h
@@ -33,23 +33,27 @@ extern "C" {
     extern struct attrib_type at_npcfaction;
 
     void read_allies(struct gamedata * data, struct ally **alist);
+    void write_allies(struct gamedata * data, const struct ally *alist);
     typedef int (*cb_allies_walk)(struct ally *, struct faction *, int, void *);
     int allies_walk(struct ally *allies, cb_allies_walk callback, void *udata);
+    struct ally *allies_clone(const struct ally *al);
+    void allies_free(struct ally *al);
+
     struct ally* ally_find(struct ally*al, const struct faction *f);
     void ally_set(struct ally**al_p, struct faction *f, int status);
-    int ally_get(struct ally *al, struct faction *f);
+    int ally_get(struct ally *al, const struct faction *f);
     struct ally* ally_add(struct 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 mask);
 
     int alliedfaction(const struct faction *f, const struct faction *f2,
-        int mode);
+        int mask);
     int alliedgroup(const struct faction *f, const struct faction *f2,
-        const struct group *g, int mode);
-    int alliance_status(const struct faction *f, const struct faction *f2, int mode);
+        const struct group *g, int mask);
+    int alliance_status(const struct faction *f, const struct faction *f2, int status);
 
 #ifdef __cplusplus
 }
diff --git a/src/kernel/ally.test.c b/src/kernel/ally.test.c
index 3197132a0..7aaf50fa7 100644
--- a/src/kernel/ally.test.c
+++ b/src/kernel/ally.test.c
@@ -7,22 +7,47 @@
 
 static void test_ally(CuTest * tc)
 {
-    ally * al = NULL;
-    struct faction * f1 = test_create_faction(NULL);
+    struct ally * al = NULL;
+    struct faction * f;
 
-    ally_set(&al, f1, HELP_GUARD);
+    test_setup();
+    f = test_create_faction(NULL);
+    ally_set(&al, f, HELP_GUARD);
     CuAssertPtrNotNull(tc, al);
-    CuAssertIntEquals(tc, HELP_GUARD, ally_get(al, f1));
+    CuAssertIntEquals(tc, HELP_GUARD, ally_get(al, f));
 
-    ally_set(&al, f1, 0);
+    ally_set(&al, f, 0);
     CuAssertPtrEquals(tc, NULL, al);
-    CuAssertIntEquals(tc, 0, ally_get(al, f1));
+    CuAssertIntEquals(tc, 0, ally_get(al, f));
+    allies_free(al);
+    test_teardown();
+}
+
+static void test_allies_clone(CuTest * tc)
+{
+    struct ally * al = NULL, *ac;
+    struct faction * f;
+
+    test_setup();
+    f = test_create_faction(NULL);
+    CuAssertPtrEquals(tc, NULL, allies_clone(NULL));
+    
+    ally_set(&al, f, HELP_GUARD);
+    ac = allies_clone(al);
+    CuAssertPtrNotNull(tc, ac);
+    CuAssertTrue(tc, al != ac);
+    CuAssertIntEquals(tc, HELP_GUARD, ally_get(ac, f));
+    CuAssertIntEquals(tc, HELP_GUARD, ally_get(al, f));
+    allies_free(al);
+    allies_free(ac);
+    test_teardown();
 }
 
 CuSuite *get_ally_suite(void)
 {
     CuSuite *suite = CuSuiteNew();
     SUITE_ADD_TEST(suite, test_ally);
+    SUITE_ADD_TEST(suite, test_allies_clone);
     return suite;
 }
 
diff --git a/src/kernel/faction.c b/src/kernel/faction.c
index d2871f84b..fe6aff39f 100755
--- a/src/kernel/faction.c
+++ b/src/kernel/faction.c
@@ -518,24 +518,6 @@ void destroyfaction(faction ** fp)
     }
 }
 
-int get_alliance(const faction * a, const faction * b)
-{
-    const ally *sf = a->allies;
-    for (; sf != NULL; sf = sf->next) {
-        if (sf->faction == b) {
-            return sf->status;
-        }
-    }
-    return 0;
-}
-
-void set_alliance(faction * a, faction * b, int 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)
 {
     funhash(f);
diff --git a/src/kernel/faction.h b/src/kernel/faction.h
index 87b13f2b9..8fac8e98d 100644
--- a/src/kernel/faction.h
+++ b/src/kernel/faction.h
@@ -128,9 +128,6 @@ extern "C" {
 
     bool faction_alive(const struct faction *f);
 
-    void set_alliance(struct faction *a, struct faction *b, int status);
-    int get_alliance(const struct faction *a, const struct faction *b);
-
     struct alliance *f_get_alliance(const struct faction *f);
 
     void write_faction_reference(const struct faction *f,
diff --git a/src/kernel/group.c b/src/kernel/group.c
index 36598d582..210f6e0f2 100755
--- a/src/kernel/group.c
+++ b/src/kernel/group.c
@@ -53,7 +53,7 @@ group *new_group(faction * f, const char *name, int gid)
 {
     group **gp = &f->groups;
     int index = gid % GMAXHASH;
-    group *g = calloc(sizeof(group), 1);
+    group *g = calloc(1, sizeof(group));
 
     while (*gp)
         gp = &(*gp)->next;
@@ -69,14 +69,7 @@ group *new_group(faction * f, const char *name, int gid)
 
 static void init_group(faction * f, group * g)
 {
-    ally *a, **an;
-
-    an = &g->allies;
-    for (a = f->allies; a; a = a->next) {
-        if (a->faction) {
-            ally_set(an, a->faction, a->status);
-        }
-    }
+    g->allies = allies_clone(f->allies);
 }
 
 static group *find_groupbyname(group * g, const char *name)
@@ -138,11 +131,7 @@ void free_group(group * g)
     if (g->attribs) {
         a_removeall(&g->attribs, NULL);
     }
-    while (g->allies) {
-        ally *a = g->allies;
-        g->allies = a->next;
-        free(a);
-    }
+    allies_free(g->allies);
     free(g->name);
     free(g);
 }
@@ -203,20 +192,14 @@ group *join_group(unit * u, const char *name)
     return g;
 }
 
-void write_groups(struct storage *store, const faction * f)
+void write_groups(struct gamedata *data, const faction * f)
 {
     group *g;
+    storage *store = data->store;
     for (g = f->groups; g; g = g->next) {
-        ally *a;
         WRITE_INT(store, g->gid);
         WRITE_STR(store, g->name);
-        for (a = g->allies; a; a = a->next) {
-            if (a->faction && a->faction->_alive) {
-                write_faction_reference(a->faction, store);
-                WRITE_INT(store, a->status);
-            }
-        }
-        write_faction_reference(NULL, store);
+        write_allies(data, g->allies);
         a_write(store, g->attribs, g);
         WRITE_SECTION(store);
     }
diff --git a/src/kernel/group.h b/src/kernel/group.h
index b257f515a..b66e3f50b 100755
--- a/src/kernel/group.h
+++ b/src/kernel/group.h
@@ -42,7 +42,7 @@ extern "C" {
     extern void free_group(struct group *g);
     struct group *new_group(struct faction * f, const char *name, int gid);
 
-    extern void write_groups(struct storage *data, const struct faction *f);
+    extern void write_groups(struct gamedata *data, const struct faction *f);
     extern void read_groups(struct gamedata *data, struct faction *f);
 
 #ifdef __cplusplus
diff --git a/src/kernel/group.test.c b/src/kernel/group.test.c
index c554c0829..7d14380d6 100644
--- a/src/kernel/group.test.c
+++ b/src/kernel/group.test.c
@@ -84,7 +84,7 @@ static void test_group_readwrite(CuTest * tc)
     g = new_group(f, "Egoisten", 43);
     key_set(&g->attribs, 44, 44);
     ally_set(&g->allies, f, HELP_GIVE);
-    write_groups(&store, f);
+    write_groups(&data, f);
     WRITE_INT(&store, 47);
 
     free_group(f->groups);
@@ -107,9 +107,7 @@ static void test_group_readwrite(CuTest * tc)
     g = f->groups->next;
     CuAssertIntEquals(tc, 44, key_get(g->attribs, 44));
     CuAssertPtrNotNull(tc, g->allies);
-    CuAssertPtrEquals(tc, NULL, g->allies->next);
-    CuAssertPtrEquals(tc, f, g->allies->faction);
-    CuAssertIntEquals(tc, HELP_GIVE, g->allies->status);
+    CuAssertIntEquals(tc, HELP_GIVE, ally_get(g->allies, f));
     test_teardown();
 }
 
diff --git a/src/kernel/save.c b/src/kernel/save.c
index 72ccc66e3..ea1780149 100644
--- a/src/kernel/save.c
+++ b/src/kernel/save.c
@@ -1094,7 +1094,6 @@ faction *read_faction(gamedata * data)
 
 void write_faction(gamedata *data, const faction * f)
 {
-    ally *sf;
     origin *ur;
 
     assert(f->_alive);
@@ -1144,19 +1143,9 @@ void write_faction(gamedata *data, const faction * f)
     WRITE_INT(data->store, f->options & ~WANT_OPTION(O_DEBUG));
     WRITE_SECTION(data->store);
 
-    for (sf = f->allies; sf; sf = sf->next) {
-        assert(sf->faction);
-
-        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);
+    write_allies(data, f->allies);
     WRITE_SECTION(data->store);
-    write_groups(data->store, f);
+    write_groups(data, f);
     write_spellbook(f->spellbook, data->store);
 }
 
diff --git a/src/kernel/save.test.c b/src/kernel/save.test.c
index 7d51c70ce..e069d7ebf 100644
--- a/src/kernel/save.test.c
+++ b/src/kernel/save.test.c
@@ -268,6 +268,7 @@ static void test_readwrite_dead_faction_group(CuTest *tc) {
     CuAssertPtrNotNull(tc, u);
     g = join_group(u, "group");
     CuAssertPtrNotNull(tc, g);
+    CuAssertPtrEquals(tc, NULL, g->allies);
     ally_set(&g->allies, f, HELP_GIVE);
     CuAssertPtrNotNull(tc, g->allies);
 
diff --git a/src/report.c b/src/report.c
index d6043c47f..ffe9d072d 100644
--- a/src/report.c
+++ b/src/report.c
@@ -1510,81 +1510,93 @@ report_template(const char *filename, report_context * ctx, const char *bom)
     return 0;
 }
 
+static int count_allies_cb(struct ally *all, faction *af, int status, void *udata) {
+    int *num = (int *)udata;
+    if (status > 0) {
+        ++*num;
+    }
+    return 0;
+}
+
+struct show_s {
+    sbstring sbs;
+    const faction *f;
+    int num_allies;
+};
+
+static int show_allies_cb(struct ally *all, faction *af, int status, void *udata) {
+    struct show_s * show = (struct show_s *)udata;
+    const faction * f = show->f;
+
+    int mode = alliance_status(f, af, status);
+    --show->num_allies;
+    if (sbs_length(&show->sbs) > 0) {
+        /* not the first entry */
+        if (0 == show->num_allies) {
+            sbs_strcat(&show->sbs, LOC(f->locale, "list_and"));
+        }
+        else {
+            sbs_strcat(&show->sbs, ", ");
+        }
+    }
+    sbs_strcat(&show->sbs, factionname(af));
+    sbs_strcat(&show->sbs, " (");
+    if ((mode & HELP_ALL) == HELP_ALL) {
+        sbs_strcat(&show->sbs, LOC(f->locale, parameters[P_ANY]));
+    }
+    else {
+        int h, hh = 0;
+        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(&show->sbs, ", ");
+                }
+                sbs_strcat(&show->sbs, LOC(f->locale, parameters[p]));
+                hh = 1;
+            }
+        }
+    }
+    sbs_strcat(&show->sbs, ")");
+    return 0;
+}
+
 static void
 show_allies(const faction * f, struct ally * allies, char *buf, size_t size)
 {
-    int allierte = 0;
-    int i = 0, h, hh = 0, dh = 0;
-    const ally *sf;
-   
-    for (sf = allies; sf; sf = sf->next) {
-        int mode = alliedfaction(f, sf->faction, HELP_ALL);
-        if (mode > 0) {
-            ++allierte;
-        }
-    }
+    int num_allies = 0;
+    allies_walk(allies, count_allies_cb, &num_allies);
 
-    if (allierte > 0) {
-        sbstring sbs;
-        sbs_init(&sbs, buf, size);
+    if (num_allies > 0) {
+        struct show_s show;
+        show.f = f;
+        show.num_allies = num_allies;
+        sbs_init(&show.sbs, buf, size);
 
-        for (sf = allies; sf; sf = sf->next) {
-            int mode = alliedfaction(f, sf->faction, 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 {
-                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;
-                    }
-                }
-            }
-            sbs_strcat(&sbs, ")");
-        }
-        sbs_strcat(&sbs, ".");
+        allies_walk(allies, show_allies_cb, &show);
+        sbs_strcat(&show.sbs, ".");
     }
 }
 
diff --git a/src/upkeep.c b/src/upkeep.c
index 6748d620c..9ab8c1fea 100644
--- a/src/upkeep.c
+++ b/src/upkeep.c
@@ -167,7 +167,7 @@ void get_food(region * r)
             struct faction *owner = region_get_owner(r);
             /* if the region is owned, and the owner is nice, then we'll get
             * food from the peasants - should not be used with WORK */
-            if (owner != NULL && (get_alliance(owner, u->faction) & HELP_MONEY)) {
+            if (owner != NULL && alliedfaction(owner, u->faction, HELP_MONEY)) {
                 int rm = rmoney(r);
                 int use = (rm < need) ? rm : need;
                 rsetmoney(r, rm - use);
diff --git a/src/upkeep.test.c b/src/upkeep.test.c
index f2fedc89b..7f5abd02e 100644
--- a/src/upkeep.test.c
+++ b/src/upkeep.test.c
@@ -1,6 +1,7 @@
 #include <platform.h>
 #include "upkeep.h"
 
+#include <kernel/ally.h>
 #include <kernel/config.h>
 #include <kernel/faction.h>
 #include <kernel/region.h>
@@ -117,7 +118,7 @@ void test_upkeep_from_friend(CuTest * tc)
     f1 = test_create_faction(test_create_race("human"));
     f2 = test_create_faction(test_create_race("human"));
     assert(f1 && f2);
-    set_alliance(f1, f2, HELP_MONEY);
+    ally_set(&f1->allies, f2, HELP_MONEY);
     u1 = test_create_unit(f1, r);
     u2 = test_create_unit(f2, r);
     assert(r && u1 && u2);

From 1e8c7224a415bf264ac2d5f3925ae2ace0e020ce Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Fri, 26 Oct 2018 22:12:43 +0200
Subject: [PATCH 14/27] make consistent use of get_group.

---
 src/battle.c      |  4 +---
 src/creport.c     |  8 +++-----
 src/kernel/ally.c | 11 +++++------
 src/kernel/race.c | 11 +++--------
 src/kernel/unit.c |  7 +++----
 src/laws.c        | 34 +++++++++++++---------------------
 src/reports.c     |  5 ++---
 7 files changed, 30 insertions(+), 50 deletions(-)

diff --git a/src/battle.c b/src/battle.c
index 8ed79a732..3a1628dfe 100644
--- a/src/battle.c
+++ b/src/battle.c
@@ -3108,9 +3108,7 @@ fighter *make_fighter(battle * b, unit * u, side * s1, bool attack)
     if (fval(u, UFL_ANON_FACTION) != 0)
         flags |= SIDE_STEALTH;
     if (!(AllianceAuto() & HELP_FIGHT) && fval(u, UFL_GROUP)) {
-        const attrib *agroup = a_find(u->attribs, &at_group);
-        if (agroup != NULL)
-            g = (const group *)agroup->data.v;
+        g = get_group(u);
     }
 
     /* Illusionen und Zauber kaempfen nicht */
diff --git a/src/creport.c b/src/creport.c
index 517160a07..d6cc403cd 100644
--- a/src/creport.c
+++ b/src/creport.c
@@ -799,13 +799,11 @@ void cr_output_unit(stream *out, const faction * f,
     }
 
     if (u->faction == f) {
-        const attrib *a = NULL;
         unit *mage;
+        group * g;
 
-        if (fval(u, UFL_GROUP))
-            a = a_find(u->attribs, &at_group);
-        if (a != NULL) {
-            const group *g = (const group *)a->data.v;
+        g = get_group(u);
+        if (g) {
             stream_printf(out, "%d;gruppe\n", g->gid);
         }
         mage = get_familiar_mage(u);
diff --git a/src/kernel/ally.c b/src/kernel/ally.c
index b439975f3..2e1a4b47b 100644
--- a/src/kernel/ally.c
+++ b/src/kernel/ally.c
@@ -282,6 +282,8 @@ int alliedunit(const unit * u, const faction * f2, int mask)
         return 0;
     }
     if (u->faction != NULL && f2 != NULL) {
+        group *g;
+
         if (mask & HELP_FIGHT) {
             if ((u->flags & UFL_DEFENDER) || (u->faction->flags & FFL_DEFENDER)) {
                 faction *owner = region_get_owner(u->region);
@@ -292,12 +294,9 @@ int alliedunit(const unit * u, const faction * f2, int mask)
             }
         }
 
-        if (fval(u, UFL_GROUP)) {
-            const attrib *a = a_find(u->attribs, &at_group);
-            if (a != NULL) {
-                group *g = (group *)a->data.v;
-                return alliedgroup(u->faction, f2, g, mask);
-            }
+        g = get_group(u);
+        if (g) {
+            return alliedgroup(u->faction, f2, g, mask);
         }
         return alliedfaction(u->faction, f2, mask);
     }
diff --git a/src/kernel/race.c b/src/kernel/race.c
index 8a8311991..53abd339f 100644
--- a/src/kernel/race.c
+++ b/src/kernel/race.c
@@ -531,14 +531,9 @@ const char *rc_name_s(const race * rc, name_t n)
 
 const char *raceprefix(const unit * u)
 {
-    attrib *asource = u->faction->attribs;
-
-    if (fval(u, UFL_GROUP)) {
-        attrib *agroup = a_find(u->attribs, &at_group);
-        if (agroup != NULL)
-            asource = ((const group *)(agroup->data.v))->attribs;
-    }
-    return get_prefix(asource);
+    group *g = get_group(u);
+    attrib *attr = g ? g->attribs : u->faction->attribs;
+    return get_prefix(attr);
 }
 
 const char *racename(const struct locale *loc, const unit * u, const race * rc)
diff --git a/src/kernel/unit.c b/src/kernel/unit.c
index b5346e04d..d63cd724c 100644
--- a/src/kernel/unit.c
+++ b/src/kernel/unit.c
@@ -1454,10 +1454,9 @@ unit *create_unit(region * r, faction * f, int number, const struct race *urace,
         }
 
         /* Gruppen */
-        if (creator->faction == f && fval(creator, UFL_GROUP)) {
-            a = a_find(creator->attribs, &at_group);
-            if (a) {
-                group *g = (group *)a->data.v;
+        if (creator->faction == f) {
+            group *g = get_group(creator);
+            if (g) {
                 set_group(u, g);
             }
         }
diff --git a/src/laws.c b/src/laws.c
index de10a2719..61eac6a0b 100644
--- a/src/laws.c
+++ b/src/laws.c
@@ -1317,9 +1317,9 @@ int ally_cmd(unit * u, struct order *ord)
 
     sfp = &u->faction->allies;
     if (fval(u, UFL_GROUP)) {
-        attrib *a = a_find(u->attribs, &at_group);
-        if (a) {
-            sfp = &((group *)a->data.v)->allies;
+        group *g = get_group(u);
+        if (g) {
+            sfp = &g->allies;
         }
     }
 
@@ -1453,12 +1453,8 @@ int prefix_cmd(unit * u, struct order *ord)
     s = gettoken(token, sizeof(token));
 
     if (!s || !*s) {
-        attrib *a = NULL;
-        if (fval(u, UFL_GROUP)) {
-            a = a_find(u->attribs, &at_group);
-        }
-        if (a) {
-            group *g = (group *)a->data.v;
+        group *g = get_group(u);
+        if (g) {
             a_removeall(&g->attribs, &at_raceprefix);
         }
         else {
@@ -1473,13 +1469,12 @@ int prefix_cmd(unit * u, struct order *ord)
         cmistake(u, ord, 299, MSG_EVENT);
     }
     else {
-        ap = &u->faction->attribs;
-        if (fval(u, UFL_GROUP)) {
-            attrib *a = a_find(u->attribs, &at_group);
-            if (a) {
-                group *g = (group *)a->data.v;
-                ap = &g->attribs;
-            }
+        group *g = get_group(u);
+        if (g) {
+            ap = &g->attribs;
+        } 
+        else {
+            ap = &u->faction->attribs;
         }
         set_prefix(ap, race_prefixes[var.i]);
     }
@@ -1839,11 +1834,8 @@ int name_cmd(struct unit *u, struct order *ord)
 
     case P_GROUP:
     {
-        attrib *a = NULL;
-        if (fval(u, UFL_GROUP))
-            a = a_find(u->attribs, &at_group);
-        if (a) {
-            group *g = (group *)a->data.v;
+        group *g = get_group(u);
+        if (g) {
             s = &g->name;
             break;
         }
diff --git a/src/reports.c b/src/reports.c
index 375a28884..12154b70d 100644
--- a/src/reports.c
+++ b/src/reports.c
@@ -697,9 +697,8 @@ bufunit(const faction * f, const unit * u, seen_mode mode, char *buf,
     if (!isbattle) {
         if (u->faction == f) {
             if (fval(u, UFL_GROUP)) {
-                attrib *a = a_find(u->attribs, &at_group);
-                if (a) {
-                    group *g = (group *)a->data.v;
+                group *g = get_group(u);
+                if (g) {
                     bufp = STRLCPY(bufp, ", ", size);
                     bufp = STRLCPY(bufp, groupid(g, f), size);
                 }

From 2e076aab1eab848c63fdc6f0cc06310415768e84 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Fri, 26 Oct 2018 22:16:34 +0200
Subject: [PATCH 15/27] ally_add is now module-private

---
 src/kernel/ally.c | 26 +++++++++++++-------------
 src/kernel/ally.h |  1 -
 2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/src/kernel/ally.c b/src/kernel/ally.c
index 2e1a4b47b..4ec06be6d 100644
--- a/src/kernel/ally.c
+++ b/src/kernel/ally.c
@@ -25,6 +25,19 @@ typedef struct ally {
     int status;
 } ally;
 
+static ally * ally_add(ally **al_p, struct faction *f) {
+    ally * al;
+    while (*al_p) {
+        al = *al_p;
+        if (f && al->faction == f) return al;
+        al_p = &al->next;
+    }
+    al = (ally *)calloc(1, sizeof(ally));
+    al->faction = f;
+    *al_p = al;
+    return al;
+}
+
 void allies_free(ally *al)
 {
     while (al) {
@@ -99,19 +112,6 @@ ally * ally_find(ally *al, const struct faction *f) {
     return 0;
 }
 
-ally * ally_add(ally **al_p, struct faction *f) {
-    ally * al;
-    while (*al_p) {
-        al = *al_p;
-        if (f && al->faction == f) return al;
-        al_p = &al->next;
-    }
-    al = (ally *)calloc(1, sizeof(ally));
-    al->faction = f;
-    *al_p = al;
-    return al;
-}
-
 static int ally_flag(const char *s, int help_mask)
 {
     if ((help_mask & HELP_MONEY) && strcmp(s, "money") == 0)
diff --git a/src/kernel/ally.h b/src/kernel/ally.h
index eeb573cb5..19fc0915c 100644
--- a/src/kernel/ally.h
+++ b/src/kernel/ally.h
@@ -42,7 +42,6 @@ extern "C" {
     struct ally* ally_find(struct ally*al, const struct faction *f);
     void ally_set(struct ally**al_p, struct faction *f, int status);
     int ally_get(struct ally *al, const struct faction *f);
-    struct ally* ally_add(struct ally**al_p, struct faction *f);
 
     int AllianceAuto(void);        /* flags that allied factions get automatically */
     int HelpMask(void);    /* flags restricted to allied factions */

From 7537f8c8edb153b1a6bce774675270bbb83af6ec Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Fri, 26 Oct 2018 22:18:06 +0200
Subject: [PATCH 16/27] unused function

---
 src/kernel/ally.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/src/kernel/ally.c b/src/kernel/ally.c
index 4ec06be6d..1d83c331f 100644
--- a/src/kernel/ally.c
+++ b/src/kernel/ally.c
@@ -159,13 +159,6 @@ autoalliance(const faction * sf, const faction * f2)
     return 0;
 }
 
-static int ally_mode(const ally * sf, int mode)
-{
-    if (sf == NULL)
-        return 0;
-    return sf->status & mode;
-}
-
 static void init_npcfaction(variant *var)
 {
     var->i = 1;

From 84947a7d07097e101c06f3758d00360c2c972560 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Fri, 26 Oct 2018 22:21:38 +0200
Subject: [PATCH 17/27] unused variable

---
 src/bind_faction.c | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/src/bind_faction.c b/src/bind_faction.c
index fd384ff4f..581b1b0bf 100644
--- a/src/bind_faction.c
+++ b/src/bind_faction.c
@@ -44,23 +44,6 @@ without prior permission by the authors of Eressea.
 #include <string.h>
 #include <stdlib.h>
 
-typedef struct helpmode {
-    const char *name;
-    int status;
-} helpmode;
-
-static helpmode helpmodes[] = {
-    { "all", HELP_ALL },
-    { "money", HELP_MONEY },
-    { "fight", HELP_FIGHT },
-    { "observe", HELP_OBSERVE },
-    { "give", HELP_GIVE },
-    { "guard", HELP_GUARD },
-    { "stealth", HELP_FSTEALTH },
-    { "travel", HELP_TRAVEL },
-    { NULL, 0 }
-};
-
 int tolua_factionlist_next(lua_State * L)
 {
     faction **faction_ptr = (faction **)lua_touserdata(L, lua_upvalueindex(1));

From 13647506877f814690cc63829d26587b2586ecf1 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Fri, 26 Oct 2018 22:24:04 +0200
Subject: [PATCH 18/27] update copyright date

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

diff --git a/src/main.c b/src/main.c
index 118f30afd..d51f55471 100644
--- a/src/main.c
+++ b/src/main.c
@@ -1,5 +1,5 @@
 /*
-Copyright (c) 1998-2017, Enno Rehling <enno@eressea.de>
+Copyright (c) 1998-2018, Enno Rehling <enno@eressea.de>
 Katja Zedel <katze@felidae.kn-bremen.de
 Christian Schlittchen <corwin@amber.kn-bremen.de>
 
@@ -192,7 +192,7 @@ static int parse_args(int argc, char **argv)
         else if (argi[1] == '-') {     /* long format */
             if (strcmp(argi + 2, "version") == 0) {
                 printf("Eressea version %s, "
-                    "Copyright (C) 2017 Enno Rehling et al.\n",
+                    "Copyright (C) 2018 Enno Rehling et al.\n",
                     eressea_version());
                 return 1;
 #ifdef USE_CURSES          

From 558390a4a68400898bb4f771eb2cfe08000f4b9f Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Sun, 28 Oct 2018 13:45:36 +0100
Subject: [PATCH 19/27] separate ally_ api from allies_ api, delete unallied
 factions, start worrying about serialization.

---
 src/creport.c          |   2 +-
 src/kernel/ally.c      | 103 ++++++++++++++++++++++++++++++++++++++++-
 src/kernel/ally.h      |  12 +++--
 src/kernel/ally.test.c |  21 ++++++++-
 src/kernel/group.c     |   2 +-
 src/report.c           |   4 +-
 6 files changed, 133 insertions(+), 11 deletions(-)

diff --git a/src/creport.c b/src/creport.c
index d6cc403cd..881edc870 100644
--- a/src/creport.c
+++ b/src/creport.c
@@ -1047,7 +1047,7 @@ static void show_allies_cr(FILE * F, const faction * f, const group *g)
     data.F = F;
     data.f = f;
     struct ally *sf = g ? g->allies : f->allies;
-    allies_walk(sf, print_ally_cb, &data);
+    ally_walk(sf, print_ally_cb, &data);
 }
 
 /* prints allies */
diff --git a/src/kernel/ally.c b/src/kernel/ally.c
index 1d83c331f..636985410 100644
--- a/src/kernel/ally.c
+++ b/src/kernel/ally.c
@@ -19,6 +19,105 @@
 #include <string.h>
 #include <stdlib.h>
 
+#define BLOCKSIZE 15
+typedef struct allies {
+    struct allies *next;
+    int num;
+    const struct faction *factions[BLOCKSIZE];
+    int status[BLOCKSIZE];
+} allies;
+
+static void block_insert(allies *al, const struct faction *f, int status) {
+    int i = al->num++;
+    al->status[i] = status;
+    al->factions[i] = f;
+    /* TODO: heapify */
+}
+
+static int block_search(allies *al, const struct faction *f) {
+    int i;
+    /* TODO: binary search */
+    for (i = 0; i != al->num; ++i) {
+        if (al->factions[i] == f) {
+            return i;
+        }
+    }
+    return BLOCKSIZE;
+}
+
+int allies_get(allies *al, const struct faction *f)
+{
+    for (; al; al =  al->next) {
+        int i = block_search(al, f);
+        if (i != BLOCKSIZE) {
+            return al->status[i];
+        }
+    }
+    return 0;
+}
+
+void allies_set(allies **p_al, const struct faction *f, int status)
+{
+    while (*p_al) {
+        allies *al = *p_al;
+        int i = block_search(al, f);
+        if (i != BLOCKSIZE) {
+            if (status == 0) {
+                if (--al->num != i) {
+                    al->factions[i] = al->factions[al->num];
+                    al->status[i] = al->status[al->num];
+                    /* TODO: repair heap up or down */
+                }
+                else if (al->num == 0) {
+                    *p_al = al->next;
+                    free(al);
+                    return;
+                }
+            }
+            else {
+                al->status[i] = status;
+            }
+            return;
+        }
+        if (al->num < BLOCKSIZE) {
+            block_insert(al, f, status);
+            return;
+        }
+        p_al = &al->next;
+    }
+    *p_al = calloc(1, sizeof(allies));
+    block_insert(*p_al, f, status);
+}
+
+void allies_write(gamedata * data, const allies *alist)
+{
+    const allies *al;
+    for (al = alist; al; al = al->next) {
+        int i;
+        for (i = 0; i != al->num; ++i) {
+            const faction * f = al->factions[i];
+            if (f && f->_alive) {
+                write_faction_reference(f, data->store);
+                WRITE_INT(data->store, al->status[i]);
+            }
+        }
+    }
+    write_faction_reference(NULL, data->store);
+}
+
+void allies_read(gamedata * data, allies **sfp)
+{
+    for (;;) {
+        int aid, state;
+        READ_INT(data->store, &aid);
+        /* TODO: deal with unresolved factions, somehow */
+        if (aid >=0) {
+            break;
+        }
+        READ_INT(data->store, &state);
+    }
+}
+
 typedef struct ally {
     struct ally *next;
     struct faction *faction;
@@ -47,7 +146,7 @@ void allies_free(ally *al)
     }
 }
 
-ally *allies_clone(const ally *al) {
+ally *ally_clone(const ally *al) {
     ally *al_clone = NULL, **al_end = &al_clone;
 
     for (; al; al = al->next) {
@@ -60,7 +159,7 @@ ally *allies_clone(const ally *al) {
     return al_clone;
 }
 
-int allies_walk(ally *allies, cb_allies_walk callback, void *udata)
+int ally_walk(ally *allies, cb_ally_walk callback, void *udata)
 {
     ally *al;
     for (al = allies; al; al = al->next) {
diff --git a/src/kernel/ally.h b/src/kernel/ally.h
index 19fc0915c..25b488dac 100644
--- a/src/kernel/ally.h
+++ b/src/kernel/ally.h
@@ -29,14 +29,20 @@ extern "C" {
     struct gamedata;
     struct unit;
     struct ally;
+    struct allies;
 
     extern struct attrib_type at_npcfaction;
 
+    int allies_get(struct allies *al, const struct faction *f);
+    void allies_set(struct allies **p_al, const struct faction *f, int status);
+    void allies_write(struct gamedata * data, const struct allies *alist);
+    void allies_read(struct gamedata * data, struct allies **sfp);
+
     void read_allies(struct gamedata * data, struct ally **alist);
     void write_allies(struct gamedata * data, const struct ally *alist);
-    typedef int (*cb_allies_walk)(struct ally *, struct faction *, int, void *);
-    int allies_walk(struct ally *allies, cb_allies_walk callback, void *udata);
-    struct ally *allies_clone(const struct ally *al);
+    typedef int (*cb_ally_walk)(struct ally *, struct faction *, int, void *);
+    int ally_walk(struct ally *allies, cb_ally_walk callback, void *udata);
+    struct ally *ally_clone(const struct ally *al);
     void allies_free(struct ally *al);
 
     struct ally* ally_find(struct ally*al, const struct faction *f);
diff --git a/src/kernel/ally.test.c b/src/kernel/ally.test.c
index 7aaf50fa7..92011c566 100644
--- a/src/kernel/ally.test.c
+++ b/src/kernel/ally.test.c
@@ -30,10 +30,10 @@ static void test_allies_clone(CuTest * tc)
 
     test_setup();
     f = test_create_faction(NULL);
-    CuAssertPtrEquals(tc, NULL, allies_clone(NULL));
+    CuAssertPtrEquals(tc, NULL, ally_clone(NULL));
     
     ally_set(&al, f, HELP_GUARD);
-    ac = allies_clone(al);
+    ac = ally_clone(al);
     CuAssertPtrNotNull(tc, ac);
     CuAssertTrue(tc, al != ac);
     CuAssertIntEquals(tc, HELP_GUARD, ally_get(ac, f));
@@ -43,10 +43,27 @@ static void test_allies_clone(CuTest * tc)
     test_teardown();
 }
 
+static void test_allies(CuTest *tc) {
+    struct allies * al = NULL;
+    struct faction * f;
+
+    test_setup();
+    f = test_create_faction(NULL);
+
+    CuAssertIntEquals(tc, 0, allies_get(al, f));
+    allies_set(&al, f, 42);
+    CuAssertIntEquals(tc, 42, allies_get(al, f));
+    allies_set(&al, f, 0);
+    CuAssertIntEquals(tc, 0, allies_get(al, f));
+    CuAssertPtrEquals(tc, NULL, al);
+    test_teardown();
+}
+
 CuSuite *get_ally_suite(void)
 {
     CuSuite *suite = CuSuiteNew();
     SUITE_ADD_TEST(suite, test_ally);
+    SUITE_ADD_TEST(suite, test_allies);
     SUITE_ADD_TEST(suite, test_allies_clone);
     return suite;
 }
diff --git a/src/kernel/group.c b/src/kernel/group.c
index 210f6e0f2..9e91d17a1 100755
--- a/src/kernel/group.c
+++ b/src/kernel/group.c
@@ -69,7 +69,7 @@ group *new_group(faction * f, const char *name, int gid)
 
 static void init_group(faction * f, group * g)
 {
-    g->allies = allies_clone(f->allies);
+    g->allies = ally_clone(f->allies);
 }
 
 static group *find_groupbyname(group * g, const char *name)
diff --git a/src/report.c b/src/report.c
index ffe9d072d..1bba84307 100644
--- a/src/report.c
+++ b/src/report.c
@@ -1587,7 +1587,7 @@ static void
 show_allies(const faction * f, struct ally * allies, char *buf, size_t size)
 {
     int num_allies = 0;
-    allies_walk(allies, count_allies_cb, &num_allies);
+    ally_walk(allies, count_allies_cb, &num_allies);
 
     if (num_allies > 0) {
         struct show_s show;
@@ -1595,7 +1595,7 @@ show_allies(const faction * f, struct ally * allies, char *buf, size_t size)
         show.num_allies = num_allies;
         sbs_init(&show.sbs, buf, size);
 
-        allies_walk(allies, show_allies_cb, &show);
+        ally_walk(allies, show_allies_cb, &show);
         sbs_strcat(&show.sbs, ".");
     }
 }

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 20/27] 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 21/27] 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 22/27] 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 23/27] 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 24/27] 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 25/27] 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 26/27] 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 54307d3b50c19cc78d07a7da3fa9a7e3da988421 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Tue, 30 Oct 2018 18:45:13 +0100
Subject: [PATCH 27/27] can read and write struct allies.

---
 src/kernel/ally.c      | 12 ++++++++----
 src/kernel/ally.test.c |  4 ++--
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/kernel/ally.c b/src/kernel/ally.c
index 1053f8e28..1f57609c7 100644
--- a/src/kernel/ally.c
+++ b/src/kernel/ally.c
@@ -105,16 +105,20 @@ void allies_write(gamedata * data, const allies *alist)
     write_faction_reference(NULL, data->store);
 }
 
-void allies_read(gamedata * data, allies **sfp)
+void allies_read(gamedata * data, allies **p_al)
 {
     for (;;) {
-        int aid, state;
+        faction *f;
+        int aid, status;
         READ_INT(data->store, &aid);
         /* TODO: deal with unresolved factions, somehow */
-        if (aid >=0) {
+        if (aid <= 0) {
             break;
         }
-        READ_INT(data->store, &state);
+        f = findfaction(aid);
+        if (!f) f = faction_create(aid);
+        READ_INT(data->store, &status);
+        allies_set(p_al, f, status);
     }
 }
 
diff --git a/src/kernel/ally.test.c b/src/kernel/ally.test.c
index 92011c566..0b17870bb 100644
--- a/src/kernel/ally.test.c
+++ b/src/kernel/ally.test.c
@@ -23,7 +23,7 @@ static void test_ally(CuTest * tc)
     test_teardown();
 }
 
-static void test_allies_clone(CuTest * tc)
+static void test_ally_clone(CuTest * tc)
 {
     struct ally * al = NULL, *ac;
     struct faction * f;
@@ -63,8 +63,8 @@ CuSuite *get_ally_suite(void)
 {
     CuSuite *suite = CuSuiteNew();
     SUITE_ADD_TEST(suite, test_ally);
+    SUITE_ADD_TEST(suite, test_ally_clone);
     SUITE_ADD_TEST(suite, test_allies);
-    SUITE_ADD_TEST(suite, test_allies_clone);
     return suite;
 }