From 526c561b901f9170576bc8197b67b10caeef638d Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Wed, 29 Oct 2014 21:08:16 +0100
Subject: [PATCH 01/10] some minor cleanup in laws.h

---
 src/bindings.c  | 16 ++++++++++++++++
 src/laws.c      | 15 ---------------
 src/laws.h      |  3 ---
 src/laws.test.c |  1 +
 4 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/src/bindings.c b/src/bindings.c
index 94caa2d42..1446dac15 100755
--- a/src/bindings.c
+++ b/src/bindings.c
@@ -68,6 +68,7 @@ without prior permission by the authors of Eressea.
 #include <quicklist.h>
 #include <util/rand.h>
 #include <util/rng.h>
+#include <util/xml.h>
 
 #include <storage.h>
 
@@ -1001,6 +1002,21 @@ static int tolua_get_spells(lua_State * L)
   return tolua_quicklist_push(L, "spell_list", "spell", spells);
 }
 
+static int init_data(const char *filename, const char *catalog)
+{
+    int l;
+    l = read_xml(filename, catalog);
+    reset_locales();
+    if (l) {
+        return l;
+    }
+    if (turn < 0) {
+        turn = first_turn;
+    }
+    return 0;
+}
+
+
 int tolua_read_xml(lua_State * L)
 {
   const char *filename = tolua_tostring(L, 1, "config.xml");
diff --git a/src/laws.c b/src/laws.c
index 038c9a550..98eefca0d 100755
--- a/src/laws.c
+++ b/src/laws.c
@@ -83,7 +83,6 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 #include <util/umlaut.h>
 #include <util/message.h>
 #include <util/rng.h>
-#include <util/xml.h>
 
 #include <attributes/otherfaction.h>
 
@@ -4557,17 +4556,3 @@ void update_subscriptions(void)
     }
     fclose(F);
 }
-
-int init_data(const char *filename, const char *catalog)
-{
-    int l;
-    l = read_xml(filename, catalog);
-    reset_locales();
-    if (l) {
-        return l;
-    }
-    if (turn < 0) {
-        turn = first_turn;
-    }
-    return 0;
-}
diff --git a/src/laws.h b/src/laws.h
index 3ffc35f08..c9fdc095f 100755
--- a/src/laws.h
+++ b/src/laws.h
@@ -24,13 +24,10 @@ extern "C" {
 
   extern int writepasswd(void);
   void demographics(void);
-  void last_orders(void);
-  void find_address(void);
   void update_guards(void);
   void update_subscriptions(void);
   void deliverMail(struct faction *f, struct region *r, struct unit *u,
     const char *s, struct unit *receiver);
-  int init_data(const char *filename, const char *catalog);
 
   bool renamed_building(const struct building * b);
   int rename_building(struct unit * u, struct order * ord, struct building * b, const char *name);
diff --git a/src/laws.test.c b/src/laws.test.c
index 48283b017..d456769b1 100644
--- a/src/laws.test.c
+++ b/src/laws.test.c
@@ -57,6 +57,7 @@ static void test_rename_building(CuTest * tc)
 
     rename_building(u, NULL, b, "Villa Nagel");
     CuAssertStrEquals(tc, "Villa Nagel", b->name);
+    CuAssertTrue(tc, renamed_building(b));
 }
 
 static void test_rename_building_twice(CuTest * tc)

From aa4a93435ed5fdf4adb1af34a1b4205ebd0853b5 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Thu, 30 Oct 2014 07:50:01 +0100
Subject: [PATCH 02/10] add missing tests for contacting during a siege.

---
 src/laws.test.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/src/laws.test.c b/src/laws.test.c
index d456769b1..43ce3dc1f 100644
--- a/src/laws.test.c
+++ b/src/laws.test.c
@@ -2,6 +2,7 @@
 #include <kernel/types.h>
 #include "laws.h"
 
+#include <kernel/ally.h>
 #include <kernel/config.h>
 #include <kernel/building.h>
 #include <kernel/faction.h>
@@ -63,9 +64,9 @@ static void test_rename_building(CuTest * tc)
 static void test_rename_building_twice(CuTest * tc)
 {
     region *r;
-    building *b;
     unit *u;
     faction *f;
+    building *b;
     building_type *btype;
 
     test_cleanup();
@@ -86,6 +87,37 @@ static void test_rename_building_twice(CuTest * tc)
     CuAssertStrEquals(tc, "Villa Kunterbunt", b->name);
 }
 
+static void test_contact(CuTest * tc)
+{
+    region *r;
+    unit *u1, *u2, *u3;
+    building *b;
+    building_type *btype;
+
+    test_cleanup();
+    test_create_world();
+
+    btype = bt_get_or_create("castle");
+    r = findregion(0, 0);
+    b = new_building(btype, r, default_locale);
+    u1 = test_create_unit(test_create_faction(0), r);
+    u2 = test_create_unit(test_create_faction(0), r);
+    u3 = test_create_unit(test_create_faction(0), r);
+    set_level(u3, SK_PERCEPTION, 2);
+    usetsiege(u3, b);
+    b->besieged = 1;
+    CuAssertIntEquals(tc, 1, can_contact(r, u1, u2));
+
+    u_set_building(u1, b);
+    CuAssertIntEquals(tc, 0, can_contact(r, u1, u2));
+    u1->faction->allies = calloc(1, sizeof(ally));
+    u1->faction->allies->faction = u2->faction;
+    u1->faction->allies->status = HELP_ALL;
+    CuAssertIntEquals(tc, HELP_GIVE, can_contact(r, u1, u2));
+    u_set_building(u2, b);
+    CuAssertIntEquals(tc, 1, can_contact(r, u1, u2));
+}
+
 static void test_fishing_feeds_2_people(CuTest * tc)
 {
     const resource_type *rtype;
@@ -413,5 +445,6 @@ CuSuite *get_laws_suite(void)
     SUITE_ADD_TEST(suite, test_reserve_cmd);
     SUITE_ADD_TEST(suite, test_new_units);
     SUITE_ADD_TEST(suite, test_cannot_create_unit_above_limit);
+    SUITE_ADD_TEST(suite, test_contact);
     return suite;
 }

From e9c13cc328a603a77724e4995013e842768b8112 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Thu, 30 Oct 2014 17:48:02 +0100
Subject: [PATCH 03/10] cosmetic changes, quick and easy assert to prevent too
 many magicians.

---
 src/kernel/pool.c |  4 +---
 src/kernel/unit.c | 11 +++++++----
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/src/kernel/pool.c b/src/kernel/pool.c
index e38c31011..5c6a942e7 100644
--- a/src/kernel/pool.c
+++ b/src/kernel/pool.c
@@ -177,11 +177,9 @@ int count)
     }
     if (rtype->flags & RTF_POOLED && mode & ~(GET_SLACK | GET_RESERVE)) {
         for (v = r->units; v && use < count; v = v->next)
-            if (u != v) {
+            if (u != v && (u->items || rtype->uget)) {
             int mask;
 
-            if (v->items == NULL && rtype->uget == NULL)
-                continue;
             if ((urace(v)->ec_flags & GIVEITEM) == 0)
                 continue;
 
diff --git a/src/kernel/unit.c b/src/kernel/unit.c
index 27dff7f0c..a039d9c3a 100644
--- a/src/kernel/unit.c
+++ b/src/kernel/unit.c
@@ -1161,10 +1161,13 @@ skill *add_skill(unit * u, skill_t id)
     ++u->skill_size;
     u->skills = realloc(u->skills, u->skill_size * sizeof(skill));
     sv = (u->skills + u->skill_size - 1);
-    sv->level = (unsigned char)0;
-    sv->weeks = (unsigned char)1;
-    sv->old = (unsigned char)0;
-    sv->id = (unsigned char)id;
+    sv->level = 0;
+    sv->weeks = 1;
+    sv->old = 0;
+    sv->id = id;
+    if (id == SK_MAGIC && u->faction) {
+        assert(max_magicians(u->faction) >= u->number);
+    }
     return sv;
 }
 

From c8d5d524125ff377594fec16ba02913e18647816 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Fri, 31 Oct 2014 07:51:57 +0100
Subject: [PATCH 04/10] add test for pool, fix a problem with (unused) feature
 that pools from allies.

---
 src/kernel/pool.c      |  2 +-
 src/kernel/pool.h      |  4 +-
 src/kernel/pool.test.c | 96 +++++++++++++++++++++++++++++++-----------
 3 files changed, 75 insertions(+), 27 deletions(-)

diff --git a/src/kernel/pool.c b/src/kernel/pool.c
index 5c6a942e7..6706ed80c 100644
--- a/src/kernel/pool.c
+++ b/src/kernel/pool.c
@@ -167,7 +167,7 @@ int count)
 
     if ((mode & GET_SLACK) && (mode & GET_RESERVE))
         use = have;
-    else {
+    else if (mode & (GET_SLACK|GET_RESERVE)) {
         int reserve = get_reservation(u, rtype);
         int slack = _max(0, have - reserve);
         if (mode & GET_RESERVE)
diff --git a/src/kernel/pool.h b/src/kernel/pool.h
index 6de15bf3c..d6f4766c6 100644
--- a/src/kernel/pool.h
+++ b/src/kernel/pool.h
@@ -29,8 +29,8 @@ extern "C" {
 #define GET_POOLED_SLACK    0x08
 #define GET_POOLED_RESERVE  0x10
 #define GET_POOLED_FORCE    0x20        /* ignore f->options pools */
-#define GET_ALLIED_SLACK    0x30
-#define GET_ALLIED_RESERVE  0x40
+#define GET_ALLIED_SLACK    0x40
+#define GET_ALLIED_RESERVE  0x80
 
 /* for convenience: */
 #define GET_DEFAULT (GET_RESERVE|GET_SLACK|GET_POOLED_SLACK)
diff --git a/src/kernel/pool.test.c b/src/kernel/pool.test.c
index ad700f6a8..8b42f1be9 100644
--- a/src/kernel/pool.test.c
+++ b/src/kernel/pool.test.c
@@ -1,47 +1,95 @@
 #include <platform.h>
 #include <kernel/types.h>
 
+#include "ally.h"
 #include "pool.h"
 #include "magic.h"
 #include "unit.h"
 #include "item.h"
+#include "faction.h"
 #include "region.h"
 #include "skill.h"
 
 #include <CuTest.h>
 #include <tests.h>
+#include <assert.h>
+
+void test_pool(CuTest *tc) {
+    unit *u1, *u2, *u3;
+    faction *f;
+    region *r;
+    struct resource_type *rtype;
+
+    test_cleanup();
+    test_create_world();
+    rtype = rt_get_or_create("money");
+    it_get_or_create(rtype);
+    f = test_create_faction(0);
+    r = findregion(0, 0);
+    assert(r && f && rtype && rtype->itype);
+    u1 = test_create_unit(f, r);
+    u2 = test_create_unit(f, r);
+    u3 = test_create_unit(test_create_faction(0), r);
+    assert(u1 && u2);
+    i_change(&u1->items, rtype->itype, 100);
+    set_resvalue(u1, rtype, 50);
+    i_change(&u2->items, rtype->itype, 200);
+    set_resvalue(u2, rtype, 100);
+    i_change(&u3->items, rtype->itype, 400);
+    set_resvalue(u3, rtype, 200);
+
+    CuAssertIntEquals(tc, 50, get_pooled(u1, rtype, GET_SLACK, 40));
+    CuAssertIntEquals(tc, 50, get_pooled(u1, rtype, GET_SLACK, INT_MAX));
+    CuAssertIntEquals(tc, 100, get_pooled(u1, rtype, GET_SLACK | GET_RESERVE, INT_MAX));
+    CuAssertIntEquals(tc, 150, get_pooled(u1, rtype, GET_SLACK | GET_POOLED_SLACK, INT_MAX));
+    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));
+
+    u3->faction->allies = calloc(1, sizeof(ally));
+    u3->faction->allies->faction = f;
+    u3->faction->allies->status = HELP_GUARD;
+    CuAssertIntEquals(tc, 0, get_pooled(u1, rtype, GET_ALLIED_SLACK | GET_ALLIED_RESERVE, INT_MAX));
+    u3->faction->allies->status = 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));
+
+    CuAssertIntEquals(tc, 100, get_pooled(u1, rtype, GET_ALL, 50));
+    CuAssertIntEquals(tc, 300, get_pooled(u1, rtype, GET_ALL, 150));
+    CuAssertIntEquals(tc, 300, get_pooled(u1, rtype, GET_ALL, INT_MAX));
+}
 
 void test_change_resource(CuTest * tc)
 {
-  struct unit * u;
-  struct faction * f;
-  struct region * r;
-  const char * names[] = { "money", "aura", "permaura", "horse", "hp", 0 };
-  int i;
+    struct unit * u;
+    struct faction * f;
+    struct region * r;
+    const char * names[] = { "money", "aura", "permaura", "horse", "hp", 0 };
+    int i;
 
-  test_cleanup();
-  test_create_world();
-  enable_skill(SK_MAGIC, true);
+    test_cleanup();
+    test_create_world();
+    enable_skill(SK_MAGIC, true);
 
-  r = findregion(0, 0);
-  f = test_create_faction(0);
-  u = test_create_unit(f, r);
-  CuAssertPtrNotNull(tc, u);
-  set_level(u, SK_MAGIC, 5);
-  create_mage(u, M_DRAIG);
+    r = findregion(0, 0);
+    f = test_create_faction(0);
+    u = test_create_unit(f, r);
+    CuAssertPtrNotNull(tc, u);
+    set_level(u, SK_MAGIC, 5);
+    create_mage(u, M_DRAIG);
 
-  for (i=0;names[i];++i) {
-    const struct resource_type *rtype = rt_find(names[i]);
-    int have = get_resource(u, rtype);
-    CuAssertIntEquals(tc, have+1, change_resource(u, rtype, 1));
-    CuAssertIntEquals(tc, have+1, get_resource(u, rtype));
-  }
+    for (i = 0; names[i]; ++i) {
+        const struct resource_type *rtype = rt_find(names[i]);
+        int have = get_resource(u, rtype);
+        CuAssertIntEquals(tc, have + 1, change_resource(u, rtype, 1));
+        CuAssertIntEquals(tc, have + 1, get_resource(u, rtype));
+    }
 }
 
 CuSuite *get_pool_suite(void)
 {
-  CuSuite *suite = CuSuiteNew();
-/*  SUITE_ADD_TEST(suite, test_pool); */
-  SUITE_ADD_TEST(suite, test_change_resource);
-  return suite;
+    CuSuite *suite = CuSuiteNew();
+    SUITE_ADD_TEST(suite, test_pool);
+    SUITE_ADD_TEST(suite, test_change_resource);
+    return suite;
 }

From f27a77d288dbb9195c3a4f027dba1ee8703b5c3b Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Fri, 31 Oct 2014 15:13:05 +0100
Subject: [PATCH 05/10] refactoring: use add_ally instead of manually crating
 the structure all over the code.

---
 src/kernel/faction.c   |   4 +-
 src/kernel/group.c     | 269 ++++++++++++++++++++---------------------
 src/kernel/pool.test.c |   8 +-
 src/kernel/save.c      |   3 +-
 src/laws.c             |   4 +-
 src/laws.test.c        |   6 +-
 6 files changed, 143 insertions(+), 151 deletions(-)

diff --git a/src/kernel/faction.c b/src/kernel/faction.c
index de9ba6ed1..5cb152e47 100755
--- a/src/kernel/faction.c
+++ b/src/kernel/faction.c
@@ -430,10 +430,8 @@ void set_alliance(faction * a, faction * b, int status)
         sfp = &sf->next;
     }
     if (*sfp == NULL) {
-        ally *sf = *sfp = malloc(sizeof(ally));
-        sf->next = NULL;
+        ally *sf = ally_add(sfp, b);
         sf->status = status;
-        sf->faction = b;
         return;
     }
     (*sfp)->status |= status;
diff --git a/src/kernel/group.c b/src/kernel/group.c
index 99e2a1a61..510c327ac 100755
--- a/src/kernel/group.c
+++ b/src/kernel/group.c
@@ -1,7 +1,7 @@
 /*
 Copyright (c) 1998-2010, Enno Rehling <enno@eressea.de>
-                         Katja Zedel <katze@felidae.kn-bremen.de
-                         Christian Schlittchen <corwin@amber.kn-bremen.de>
+Katja Zedel <katze@felidae.kn-bremen.de
+Christian Schlittchen <corwin@amber.kn-bremen.de>
 
 Permission to use, copy, modify, and/or distribute this software for any
 purpose with or without fee is hereby granted, provided that the above
@@ -50,203 +50,200 @@ static int maxgid;
 
 static 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 **gp = &f->groups;
+    int index = gid % GMAXHASH;
+    group *g = calloc(sizeof(group), 1);
 
-  while (*gp)
-    gp = &(*gp)->next;
-  *gp = g;
+    while (*gp)
+        gp = &(*gp)->next;
+    *gp = g;
 
-  maxgid = _max(gid, maxgid);
-  g->name = _strdup(name);
-  g->gid = gid;
+    maxgid = _max(gid, maxgid);
+    g->name = _strdup(name);
+    g->gid = gid;
 
-  g->nexthash = ghash[index];
-  return ghash[index] = g;
+    g->nexthash = ghash[index];
+    return ghash[index] = g;
 }
 
 static void init_group(faction * f, group * g)
 {
-  ally *a, **an;
+    ally *a, **an;
 
-  an = &g->allies;
-  for (a = f->allies; a; a = a->next)
-    if (a->faction) {
-      ally *ga = calloc(sizeof(ally), 1);
-      *ga = *a;
-      *an = ga;
-      an = &ga->next;
-    }
+    an = &g->allies;
+    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;
+        }
 }
 
 static group *find_groupbyname(group * g, const char *name)
 {
-  while (g && unicode_utf8_strcasecmp(name, g->name) != 0)
-    g = g->next;
-  return g;
+    while (g && unicode_utf8_strcasecmp(name, g->name) != 0)
+        g = g->next;
+    return g;
 }
 
 static group *find_group(int gid)
 {
-  int index = gid % GMAXHASH;
-  group *g = ghash[index];
-  while (g && g->gid != gid)
-    g = g->nexthash;
-  return g;
+    int index = gid % GMAXHASH;
+    group *g = ghash[index];
+    while (g && g->gid != gid)
+        g = g->nexthash;
+    return g;
 }
 
 static int read_group(attrib * a, void *owner, struct storage *store)
 {
-  group *g;
-  int gid;
+    group *g;
+    int gid;
 
-  READ_INT(store, &gid);
-  a->data.v = g = find_group(gid);
-  if (g != 0) {
-    g->members++;
-    return AT_READ_OK;
-  }
-  return AT_READ_FAIL;
+    READ_INT(store, &gid);
+    a->data.v = g = find_group(gid);
+    if (g != 0) {
+        g->members++;
+        return AT_READ_OK;
+    }
+    return AT_READ_FAIL;
 }
 
 static void
 write_group(const attrib * a, const void *owner, struct storage *store)
 {
-  group *g = (group *) a->data.v;
-  WRITE_INT(store, g->gid);
+    group *g = (group *)a->data.v;
+    WRITE_INT(store, g->gid);
 }
 
 attrib_type at_group = {        /* attribute for units assigned to a group */
-"grp",
+    "grp",
     DEFAULT_INIT,
-    DEFAULT_FINALIZE, DEFAULT_AGE, write_group, read_group, ATF_UNIQUE};
+    DEFAULT_FINALIZE, DEFAULT_AGE, write_group, read_group, ATF_UNIQUE };
 
 void free_group(group * g)
 {
-  int index = g->gid % GMAXHASH;
-  group **g_ptr = ghash + index;
-  while (*g_ptr && (*g_ptr)->gid != g->gid)
-    g_ptr = &(*g_ptr)->nexthash;
-  assert(*g_ptr == g);
-  *g_ptr = g->nexthash;
+    int index = g->gid % GMAXHASH;
+    group **g_ptr = ghash + index;
+    while (*g_ptr && (*g_ptr)->gid != g->gid)
+        g_ptr = &(*g_ptr)->nexthash;
+    assert(*g_ptr == g);
+    *g_ptr = g->nexthash;
 
-  while (g->allies) {
-    ally *a = g->allies;
-    g->allies = a->next;
-    free(a);
-  }
-  free(g->name);
-  free(g);
+    while (g->allies) {
+        ally *a = g->allies;
+        g->allies = a->next;
+        free(a);
+    }
+    free(g->name);
+    free(g);
 }
 
 group * get_group(const struct unit *u)
 {
-  if (fval(u, UFL_GROUP)) {
-    attrib * a = a_find(u->attribs, &at_group);
-    if (a) {
-      return (group *) a->data.v;
+    if (fval(u, UFL_GROUP)) {
+        attrib * a = a_find(u->attribs, &at_group);
+        if (a) {
+            return (group *)a->data.v;
+        }
     }
-  }
-  return 0;
+    return 0;
 }
 
 void set_group(struct unit *u, struct group *g)
 {
-  attrib *a = NULL;
+    attrib *a = NULL;
 
-  if (fval(u, UFL_GROUP)) {
-    a = a_find(u->attribs, &at_group);
-  }
-
-  if (a) {
-    group *og = (group *) a->data.v;
-    if (og == g)
-      return;
-    --og->members;
-  }
-
-  if (g) {
-    if (!a) {
-      a = a_add(&u->attribs, a_new(&at_group));
-      fset(u, UFL_GROUP);
+    if (fval(u, UFL_GROUP)) {
+        a = a_find(u->attribs, &at_group);
+    }
+
+    if (a) {
+        group *og = (group *)a->data.v;
+        if (og == g)
+            return;
+        --og->members;
+    }
+
+    if (g) {
+        if (!a) {
+            a = a_add(&u->attribs, a_new(&at_group));
+            fset(u, UFL_GROUP);
+        }
+        a->data.v = g;
+        g->members++;
+    }
+    else if (a) {
+        a_remove(&u->attribs, a);
+        freset(u, UFL_GROUP);
     }
-    a->data.v = g;
-    g->members++;
-  } else if (a) {
-    a_remove(&u->attribs, a);
-    freset(u, UFL_GROUP);
-  }
 }
 
 bool join_group(unit * u, const char *name)
 {
-  group *g = NULL;
+    group *g = NULL;
 
-  if (name && name[0]) {
-    g = find_groupbyname(u->faction->groups, name);
-    if (g == NULL) {
-      g = new_group(u->faction, name, ++maxgid);
-      init_group(u->faction, g);
+    if (name && name[0]) {
+        g = find_groupbyname(u->faction->groups, name);
+        if (g == NULL) {
+            g = new_group(u->faction, name, ++maxgid);
+            init_group(u->faction, g);
+        }
     }
-  }
 
-  set_group(u, g);
-  return true;
+    set_group(u, g);
+    return true;
 }
 
 void write_groups(struct storage *store, group * g)
 {
-  while (g) {
-    ally *a;
-    WRITE_INT(store, g->gid);
-    WRITE_STR(store, g->name);
-    for (a = g->allies; a; a = a->next) {
-      if (a->faction) {
-        write_faction_reference(a->faction, store);
-        WRITE_INT(store, a->status);
-      }
+    while (g) {
+        ally *a;
+        WRITE_INT(store, g->gid);
+        WRITE_STR(store, g->name);
+        for (a = g->allies; a; a = a->next) {
+            if (a->faction) {
+                write_faction_reference(a->faction, store);
+                WRITE_INT(store, a->status);
+            }
+        }
+        WRITE_INT(store, 0);
+        a_write(store, g->attribs, g);
+        WRITE_SECTION(store);
+        g = g->next;
     }
     WRITE_INT(store, 0);
-    a_write(store, g->attribs, g);
-    WRITE_SECTION(store);
-    g = g->next;
-  }
-  WRITE_INT(store, 0);
 }
 
 void read_groups(struct storage *store, faction * f)
 {
-  for (;;) {
-    ally **pa;
-    group *g;
-    int gid;
-    char buf[1024];
-
-    READ_INT(store, &gid);
-    if (gid == 0)
-      break;
-    READ_STR(store, buf, sizeof(buf));
-    g = new_group(f, buf, gid);
-    pa = &g->allies;
     for (;;) {
-      ally *a;
-      variant fid;
-      READ_INT(store, &fid.i);
-      if (fid.i <= 0)
-        break;
-      if (global.data_version < STORAGE_VERSION && fid.i == 0)
-        break;
-      a = malloc(sizeof(ally));
-      *pa = a;
-      pa = &a->next;
-      READ_INT(store, &a->status);
+        ally **pa;
+        group *g;
+        int gid;
+        char buf[1024];
 
-      a->faction = findfaction(fid.i);
-      if (!a->faction)
-        ur_add(fid, &a->faction, resolve_faction);
+        READ_INT(store, &gid);
+        if (gid == 0)
+            break;
+        READ_STR(store, buf, sizeof(buf));
+        g = new_group(f, buf, gid);
+        pa = &g->allies;
+        for (;;) {
+            ally *a;
+            variant fid;
+
+            READ_INT(store, &fid.i);
+            if (fid.i <= 0)
+                break;
+            if (global.data_version < STORAGE_VERSION && fid.i == 0)
+                break;
+            a = ally_add(pa, findfaction(fid.i));
+            READ_INT(store, &a->status);
+            if (!a->faction)
+                ur_add(fid, &a->faction, resolve_faction);
+        }
+        *pa = 0;
+        a_read(store, &g->attribs, g);
     }
-    *pa = 0;
-    a_read(store, &g->attribs, g);
-  }
 }
diff --git a/src/kernel/pool.test.c b/src/kernel/pool.test.c
index 8b42f1be9..1a0432460 100644
--- a/src/kernel/pool.test.c
+++ b/src/kernel/pool.test.c
@@ -19,6 +19,7 @@ void test_pool(CuTest *tc) {
     faction *f;
     region *r;
     struct resource_type *rtype;
+    ally *al;
 
     test_cleanup();
     test_create_world();
@@ -45,11 +46,10 @@ 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));
 
-    u3->faction->allies = calloc(1, sizeof(ally));
-    u3->faction->allies->faction = f;
-    u3->faction->allies->status = HELP_GUARD;
+    al = ally_add(&u3->faction->allies, f);
+    al->status = HELP_GUARD;
     CuAssertIntEquals(tc, 0, get_pooled(u1, rtype, GET_ALLIED_SLACK | GET_ALLIED_RESERVE, INT_MAX));
-    u3->faction->allies->status = HELP_MONEY;
+    al->status = 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));
diff --git a/src/kernel/save.c b/src/kernel/save.c
index 6f443598a..77a33b50f 100644
--- a/src/kernel/save.c
+++ b/src/kernel/save.c
@@ -1119,8 +1119,7 @@ static ally **addally(const faction * f, ally ** sfp, int aid, int state)
     if (state == 0)
         return sfp;
 
-    sf = calloc(1, sizeof(ally));
-    sf->faction = af;
+    sf = ally_add(sfp, af);
     if (!sf->faction) {
         variant id;
         id.i = aid;
diff --git a/src/laws.c b/src/laws.c
index 98eefca0d..ed319023e 100755
--- a/src/laws.c
+++ b/src/laws.c
@@ -1348,10 +1348,8 @@ int ally_cmd(unit * u, struct order *ord)
             return 0;
         }
         else {
-            sf = calloc(1, sizeof(ally));
-            sf->faction = f;
+            sf = ally_add(sfp, f);
             sf->status = 0;
-            addlist(sfp, sf);
         }
     }
     switch (keyword) {
diff --git a/src/laws.test.c b/src/laws.test.c
index 43ce3dc1f..6549d1bf6 100644
--- a/src/laws.test.c
+++ b/src/laws.test.c
@@ -93,6 +93,7 @@ static void test_contact(CuTest * tc)
     unit *u1, *u2, *u3;
     building *b;
     building_type *btype;
+    ally *al;
 
     test_cleanup();
     test_create_world();
@@ -110,9 +111,8 @@ static void test_contact(CuTest * tc)
 
     u_set_building(u1, b);
     CuAssertIntEquals(tc, 0, can_contact(r, u1, u2));
-    u1->faction->allies = calloc(1, sizeof(ally));
-    u1->faction->allies->faction = u2->faction;
-    u1->faction->allies->status = HELP_ALL;
+    al = ally_add(&u1->faction->allies, u2->faction);
+    al->status = HELP_ALL;
     CuAssertIntEquals(tc, HELP_GIVE, can_contact(r, u1, u2));
     u_set_building(u2, b);
     CuAssertIntEquals(tc, 1, can_contact(r, u1, u2));

From cf110d7788eff7a581f2ea362df410c1f859077a Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Fri, 31 Oct 2014 15:38:37 +0100
Subject: [PATCH 06/10] fix allies with null faction (to be resolved later).

---
 src/kernel/ally.c      | 52 ++++++++++++++++++++++--------------------
 src/kernel/ally.test.c | 36 ++++++++++++++++++++---------
 2 files changed, 52 insertions(+), 36 deletions(-)

diff --git a/src/kernel/ally.c b/src/kernel/ally.c
index cf5772fd6..3ebc7c1ce 100644
--- a/src/kernel/ally.c
+++ b/src/kernel/ally.c
@@ -4,36 +4,38 @@
 #include <stdlib.h>
 
 ally * ally_find(ally *al, const struct faction *f) {
-  for (;al;al=al->next) {
-    if (al->faction==f) return al;
-  }
-  return 0;
+    for (; al; al = al->next) {
+        if (al->faction == f) return al;
+    }
+    return 0;
 }
 
 ally * ally_add(ally **al_p, struct faction *f) {
-  ally * al;
-  while (*al_p) {
-    al = *al_p;
-    if (al->faction==f) return al;
-    al_p = &al->next;
-  }
-  al = (ally *)malloc(sizeof(ally));
-  al->faction = f;
-  al->status = 0;
-  al->next = 0;
-  *al_p = al;
-  return al;
+    ally * al;
+    if (f) {
+        while (*al_p) {
+            al = *al_p;
+            if (al->faction == f) return al;
+            al_p = &al->next;
+        }
+    }
+    al = (ally *)malloc(sizeof(ally));
+    al->faction = f;
+    al->status = 0;
+    al->next = 0;
+    *al_p = al;
+    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;
+    ally * al;
+    while (*al_p) {
+        al = *al_p;
+        if (al->faction == f) {
+            *al_p = al->next;
+            free(al);
+            break;
+        }
+        al_p = &al->next;
     }
-    al_p = &al->next;
-  }
 }
diff --git a/src/kernel/ally.test.c b/src/kernel/ally.test.c
index 7a3155159..fb35d25f7 100644
--- a/src/kernel/ally.test.c
+++ b/src/kernel/ally.test.c
@@ -7,22 +7,36 @@
 
 static void test_ally(CuTest * tc)
 {
-  ally * al = 0;
-  struct faction * f1 = test_create_faction(0);
+    ally * al = 0;
+    struct faction * f1 = test_create_faction(0);
 
-  ally_add(&al, f1);
-  CuAssertPtrNotNull(tc, al);
-  CuAssertPtrEquals(tc, f1, ally_find(al, f1)->faction);
+    ally_add(&al, f1);
+    CuAssertPtrNotNull(tc, al);
+    CuAssertPtrEquals(tc, f1, ally_find(al, f1)->faction);
 
-  ally_remove(&al, f1);
-  CuAssertPtrEquals(tc, 0, al);
-  CuAssertPtrEquals(tc, 0, ally_find(al, f1));
+    ally_remove(&al, f1);
+    CuAssertPtrEquals(tc, 0, al);
+    CuAssertPtrEquals(tc, 0, 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, 0, a2->next);
+    free(a1);
+    free(a2);
 }
 
 CuSuite *get_ally_suite(void)
 {
-  CuSuite *suite = CuSuiteNew();
-  SUITE_ADD_TEST(suite, test_ally);
-  return suite;
+    CuSuite *suite = CuSuiteNew();
+    SUITE_ADD_TEST(suite, test_ally);
+    return suite;
 }
 

From 838155179f599caeb127c3349b3f7a1bf3ac9f73 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Fri, 31 Oct 2014 15:38:53 +0100
Subject: [PATCH 07/10]  fix backwards compatible code for at_permission.

---
 src/modules/gmcmd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/modules/gmcmd.c b/src/modules/gmcmd.c
index 9a5085ecf..d98ae9dbe 100644
--- a/src/modules/gmcmd.c
+++ b/src/modules/gmcmd.c
@@ -45,9 +45,9 @@
 
 static int read_permissions(attrib * a, void *owner, struct storage *store)
 {
-  attrib *attr = NULL;
-  a_read(store, &attr, owner);
-  a_remove(&attr, a);
+  assert(!a);
+  a_read(store, &a, owner);
+  a_remove(&a, a);
   return AT_READ_OK;
 }
 

From 997d1ffd4b9dbcf3c3a671a33d4e7c10e640838d Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Fri, 31 Oct 2014 15:52:40 +0100
Subject: [PATCH 08/10] Fix some crashes that happened while debugging an old
 data file.

---
 src/kernel/save.c | 7 ++++---
 src/reports.c     | 2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/kernel/save.c b/src/kernel/save.c
index 77a33b50f..f6fb4c540 100644
--- a/src/kernel/save.c
+++ b/src/kernel/save.c
@@ -1119,6 +1119,10 @@ static ally **addally(const faction * f, ally ** sfp, int aid, int state)
     if (state == 0)
         return sfp;
 
+    while (*sfp) {
+        sfp = &(*sfp)->next;
+    }
+
     sf = ally_add(sfp, af);
     if (!sf->faction) {
         variant id;
@@ -1127,9 +1131,6 @@ static ally **addally(const faction * f, ally ** sfp, int aid, int state)
     }
     sf->status = state & HELP_ALL;
 
-    while (*sfp)
-        sfp = &(*sfp)->next;
-    *sfp = sf;
     return &sf->next;
 }
 
diff --git a/src/reports.c b/src/reports.c
index 1939fa972..e7ca9c314 100644
--- a/src/reports.c
+++ b/src/reports.c
@@ -1183,7 +1183,7 @@ link_seen(seen_region * seehash[], const region * first, const region * last)
         }
         r = r->next;
     }
-    sr->next = 0;
+    if (sr) sr->next = 0;
 }
 
 seen_region *find_seen(struct seen_region *seehash[], const region * r)

From 9848ba08d00cdbd3f1324a04f9677f2fc6f700a4 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Fri, 31 Oct 2014 15:54:25 +0100
Subject: [PATCH 09/10] fix gcc and clang builds: missing include

---
 src/kernel/pool.test.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/kernel/pool.test.c b/src/kernel/pool.test.c
index 1a0432460..7a45475db 100644
--- a/src/kernel/pool.test.c
+++ b/src/kernel/pool.test.c
@@ -13,6 +13,7 @@
 #include <CuTest.h>
 #include <tests.h>
 #include <assert.h>
+#include <limits.h>
 
 void test_pool(CuTest *tc) {
     unit *u1, *u2, *u3;

From 6e24fe9be29c213d816b87fc907fc1600f5af480 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Fri, 31 Oct 2014 16:51:57 +0100
Subject: [PATCH 10/10] fix gcc build, ebable new test.

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

diff --git a/src/kernel/ally.c b/src/kernel/ally.c
index 3ebc7c1ce..ed53cbce4 100644
--- a/src/kernel/ally.c
+++ b/src/kernel/ally.c
@@ -12,12 +12,10 @@ ally * ally_find(ally *al, const struct faction *f) {
 
 ally * ally_add(ally **al_p, struct faction *f) {
     ally * al;
-    if (f) {
-        while (*al_p) {
-            al = *al_p;
-            if (al->faction == f) return al;
-            al_p = &al->next;
-        }
+    while (*al_p) {
+        al = *al_p;
+        if (f && al->faction == f) return al;
+        al_p = &al->next;
     }
     al = (ally *)malloc(sizeof(ally));
     al->faction = f;
diff --git a/src/kernel/ally.test.c b/src/kernel/ally.test.c
index fb35d25f7..c5c47298f 100644
--- a/src/kernel/ally.test.c
+++ b/src/kernel/ally.test.c
@@ -37,6 +37,7 @@ CuSuite *get_ally_suite(void)
 {
     CuSuite *suite = CuSuiteNew();
     SUITE_ADD_TEST(suite, test_ally);
+    SUITE_ADD_TEST(suite, test_ally_null);
     return suite;
 }