From de512be1ba0fe844309926ad93355bfe71903c11 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Mon, 11 Jan 2016 09:55:47 +0100
Subject: [PATCH 1/3] make destroyfaction remove the faction from the list
 (slightly scary change)

---
 src/bind_faction.c        |  10 +++-
 src/kernel/alliance.c     |  11 ++--
 src/kernel/faction.c      | 102 +++++++++++++-------------------------
 src/kernel/faction.h      |   4 +-
 src/kernel/faction.test.c |   2 +-
 src/laws.c                |  52 +++++++++----------
 6 files changed, 79 insertions(+), 102 deletions(-)

diff --git a/src/bind_faction.c b/src/bind_faction.c
index dc06c4057..19f4c8989 100644
--- a/src/bind_faction.c
+++ b/src/bind_faction.c
@@ -343,8 +343,14 @@ static int tolua_faction_get_origin(lua_State * L)
 
 static int tolua_faction_destroy(lua_State * L)
 {
-    faction *f = (faction *)tolua_tousertype(L, 1, 0);
-    destroyfaction(f);
+    faction **fp, *f = (faction *)tolua_tousertype(L, 1, 0);
+    // TODO: this loop is slow af, but what can we do?
+    for (fp = &factions; *fp; fp = &(*fp)->next) {
+        if (*fp == f) {
+            destroyfaction(fp);
+            break;
+        }
+    }
     return 0;
 }
 
diff --git a/src/kernel/alliance.c b/src/kernel/alliance.c
index 76f37edce..1de816731 100644
--- a/src/kernel/alliance.c
+++ b/src/kernel/alliance.c
@@ -424,13 +424,14 @@ void alliancevictory(void)
     }
     while (al != NULL) {
         if (!fval(al, FFL_MARK)) {
-            int qi;
-            quicklist *flist = al->members;
-            for (qi = 0; flist; ql_advance(&flist, &qi, 1)) {
-                faction *f = (faction *)ql_get(flist, qi);
+            faction **fp;
+            for (fp = &factions; *fp; ) {
+                faction *f = *fp;
                 if (f->alliance == al) {
                     ADDMSG(&f->msgs, msg_message("alliance::lost", "alliance", al));
-                    destroyfaction(f);
+                    destroyfaction(fp);
+                } else {
+                    fp = &f->next;
                 }
             }
         }
diff --git a/src/kernel/faction.c b/src/kernel/faction.c
index 1ca85aa8d..28b2719b3 100755
--- a/src/kernel/faction.c
+++ b/src/kernel/faction.c
@@ -252,7 +252,7 @@ faction *addfaction(const char *email, const char *password,
 
     f->alliance_joindate = turn;
     f->lastorders = turn;
-    f->alive = 1;
+    f->_alive = true;
     f->age = 0;
     f->race = frace;
     f->magiegebiet = 0;
@@ -322,17 +322,16 @@ variant read_faction_reference(struct storage * store)
 
 void write_faction_reference(const faction * f, struct storage *store)
 {
-    WRITE_INT(store, (f && f->alive) ? f->no : 0);
+    WRITE_INT(store, (f && f->_alive) ? f->no : 0);
 }
 
-void destroyfaction(faction * f)
+void destroyfaction(faction ** fp)
 {
+    faction * f = *fp;
     unit *u = f->units;
     faction *ff;
 
-    if (!f->alive) {
-        return;
-    }
+    *fp = f->next;
     fset(f, FFL_QUIT);
 
     if (f->spellbook) {
@@ -389,32 +388,43 @@ void destroyfaction(faction * f)
             u = u->nextF;
         }
     }
-    f->alive = 0;
+    f->alive = false;
     /* no way!  f->units = NULL; */
     handle_event(f->attribs, "destroy", f);
     for (ff = factions; ff; ff = ff->next) {
         group *g;
-        ally *sf, *sfn;
+        ally *sf, **sfp;
 
-        /* Alle HELFE für die Partei löschen */
-        for (sf = ff->allies; sf; sf = sf->next) {
-            if (sf->faction == f) {
-                removelist(&ff->allies, sf);
-                break;
+        for (sfp = &ff->allies; *sfp;) {
+            sf = *sfp;
+            if (sf->faction == f || sf->faction == NULL) {
+                *sfp = sf->next;
+                free(sf);
             }
+            else
+                sfp = &(*sfp)->next;
         }
         for (g = ff->groups; g; g = g->next) {
-            for (sf = g->allies; sf;) {
-                sfn = sf->next;
-                if (sf->faction == f) {
-                    removelist(&g->allies, sf);
-                    break;
+            for (sfp = &g->allies; *sfp; ) {
+                sf = *sfp;
+                if (sf->faction == f || sf->faction == NULL) {
+                    *sfp = sf->next;
+                    free(sf);
+                }
+                else {
+                    sfp = &(*sfp)->next;
                 }
-                sf = sfn;
             }
         }
     }
 
+    if (f->alliance && f->alliance->_leader == f) {
+        setalliance(f, 0);
+    }
+
+    funhash(f);
+    free_faction(f);
+
     /* units of other factions that were disguised as this faction
      * have their disguise replaced by ordinary faction hiding. */
     if (rule_stealth_other()) {
@@ -633,60 +643,18 @@ int skill_limit(faction * f, skill_t sk)
 
 void remove_empty_factions(void)
 {
-    faction **fp, *f3;
+    faction **fp;
 
     for (fp = &factions; *fp;) {
         faction *f = *fp;
-        /* monster (0) werden nicht entfernt. alive kann beim readgame
-        * () auf 0 gesetzt werden, wenn monsters keine einheiten mehr
-        * haben. */
-        if ((f->units == NULL || f->alive == 0) && !fval(f, FFL_NOIDLEOUT)) {
-            ursprung *ur = f->ursprung;
-            while (ur && ur->id != 0)
-                ur = ur->next;
+
+        if ((!f->alive || !f->units) && !fval(f, FFL_NOIDLEOUT)) {
             log_debug("dead: %s", factionname(f));
-
-            /* Einfach in eine Datei schreiben und später vermailen */
-
-            for (f3 = factions; f3; f3 = f3->next) {
-                ally *sf;
-                group *g;
-                ally **sfp = &f3->allies;
-                while (*sfp) {
-                    sf = *sfp;
-                    if (sf->faction == f || sf->faction == NULL) {
-                        *sfp = sf->next;
-                        free(sf);
-                    }
-                    else
-                        sfp = &(*sfp)->next;
-                }
-                for (g = f3->groups; g; g = g->next) {
-                    sfp = &g->allies;
-                    while (*sfp) {
-                        sf = *sfp;
-                        if (sf->faction == f || sf->faction == NULL) {
-                            *sfp = sf->next;
-                            free(sf);
-                        }
-                        else
-                            sfp = &(*sfp)->next;
-                    }
-                }
-            }
-
-            *fp = f->next;
-            funhash(f);
-            free_faction(f);
-            if (f->alliance && f->alliance->_leader == f) {
-                setalliance(f, 0);
-            }
-            destroyfaction(f); // TODO: there was a free() here,
-            // are we duplicating efforts here that also happen
-            // in destroyfaction?
+            destroyfaction(fp);
         }
-        else
+        else {
             fp = &(*fp)->next;
+        }
     }
 }
 
diff --git a/src/kernel/faction.h b/src/kernel/faction.h
index d22763d96..6692636f0 100644
--- a/src/kernel/faction.h
+++ b/src/kernel/faction.h
@@ -103,7 +103,7 @@ extern "C" {
         struct item *items;         /* items this faction can claim */
         struct seen_region **seen;
         struct quicklist *seen_factions;
-        bool alive;              /* enno: sollte ein flag werden */
+        bool _alive;              /* enno: sollte ein flag werden */
     } faction;
 
     extern struct faction *factions;
@@ -121,7 +121,7 @@ extern "C" {
     struct faction *addfaction(const char *email, const char *password,
         const struct race *frace, const struct locale *loc, int subscription);
     bool checkpasswd(const faction * f, const char *passwd);
-    void destroyfaction(faction * f);
+    void destroyfaction(faction ** f);
 
     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/faction.test.c b/src/kernel/faction.test.c
index d1a5c53be..6b456c9ac 100644
--- a/src/kernel/faction.test.c
+++ b/src/kernel/faction.test.c
@@ -75,7 +75,7 @@ static void test_remove_dead_factions(CuTest *tc) {
     remove_empty_factions();
     CuAssertPtrEquals(tc, f, findfaction(f->no));
     CuAssertPtrNotNull(tc, get_monsters());
-    fm->alive = 0;
+    fm->units = 0;
     f->alive = 0;
     fno = f->no;
     remove_empty_factions();
diff --git a/src/laws.c b/src/laws.c
index c88145760..ec67c9982 100755
--- a/src/laws.c
+++ b/src/laws.c
@@ -1220,42 +1220,45 @@ static void nmr_death(faction * f)
 
 static void remove_idle_players(void)
 {
-    faction *f;
+    faction **fp;
 
     log_info(" - beseitige Spieler, die sich zu lange nicht mehr gemeldet haben...");
 
-    for (f = factions; f; f = f->next) {
+    for (fp = &factions; *fp;) {
+        faction *f = *fp;
         if (fval(f, FFL_NOIDLEOUT)) {
             f->lastorders = turn;
         }
         if (NMRTimeout() > 0 && turn - f->lastorders >= NMRTimeout()) {
             nmr_death(f);
-            destroyfaction(f);
-            continue;
-        }
-        if (turn != f->lastorders) {
+            destroyfaction(fp);
+        } else if (turn != f->lastorders) {
             char info[256];
             sprintf(info, "%d Einheiten, %d Personen, %d Silber",
                 f->no_units, f->num_total, f->money);
         }
+        fp = &f->next;
     }
     log_info(" - beseitige Spieler, die sich nach der Anmeldung nicht gemeldet haben...");
 
     age = calloc(_max(4, turn + 1), sizeof(int));
-    for (f = factions; f; f = f->next)
-        if (!is_monsters(f)) {
+    for (fp = &factions; *fp;) {
+        faction *f = *fp;
+        if (f->alive && !is_monsters(f)) {
             if (RemoveNMRNewbie() && !fval(f, FFL_NOIDLEOUT)) {
                 if (f->age >= 0 && f->age <= turn)
                     ++age[f->age];
                 if (f->age == 2 || f->age == 3) {
                     if (f->lastorders == turn - 2) {
-                        destroyfaction(f);
                         ++dropouts[f->age - 2];
+                        destroyfaction(fp);
                         continue;
                     }
                 }
             }
         }
+        fp = &f->next;
+    }
 }
 
 void quit(void)
@@ -1264,7 +1267,7 @@ void quit(void)
     while (*fptr) {
         faction *f = *fptr;
         if (f->flags & FFL_QUIT) {
-            destroyfaction(f);
+            destroyfaction(fptr);
         }
         else {
             ++f->age;
@@ -1272,8 +1275,6 @@ void quit(void)
                 ADDMSG(&f->msgs, msg_message("newbieimmunity", "turns",
                     NewbieImmunity() - f->age - 1));
             }
-        }
-        if (*fptr == f) {
             fptr = &f->next;
         }
     }
@@ -4263,21 +4264,22 @@ static void maintain_buildings_1(region * r)
  */
 static int warn_password(void)
 {
-    faction *f = factions;
-    while (f) {
+    faction *f;
+    for (f = factions; f; f = f->next) {
         bool pwok = true;
-        const char *c = f->passw;
-        while (*c && pwok) {
-            if (!isalnum((unsigned char)*c))
-                pwok = false;
-            c++;
+        if (f->alive) {
+            const char *c = f->passw;
+            while (*c && pwok) {
+                if (!isalnum((unsigned char)*c))
+                    pwok = false;
+                c++;
+            }
+            if (!pwok) {
+                free(f->passw);
+                f->passw = _strdup(itoa36(rng_int()));
+                ADDMSG(&f->msgs, msg_message("illegal_password", "newpass", f->passw));
+            }
         }
-        if (!pwok) {
-            free(f->passw);
-            f->passw = _strdup(itoa36(rng_int()));
-            ADDMSG(&f->msgs, msg_message("illegal_password", "newpass", f->passw));
-        }
-        f = f->next;
     }
     return 0;
 }

From b4389c91fe0c9f62455018e951b25ddcaf2a1d0a Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Mon, 11 Jan 2016 11:54:45 +0100
Subject: [PATCH 2/3] fix compilation (what was I thinking?), remove uses of
 f.alive where they should not be needed.

---
 src/kernel/faction.c      |  4 ++--
 src/kernel/faction.test.c |  4 ++--
 src/kernel/save.c         |  4 ++--
 src/kernel/unit.c         |  4 ++--
 src/kernel/unit.test.c    |  2 +-
 src/laws.c                | 24 +++++++++++-------------
 src/summary.c             |  4 ++--
 7 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/src/kernel/faction.c b/src/kernel/faction.c
index 28b2719b3..0ab13c150 100755
--- a/src/kernel/faction.c
+++ b/src/kernel/faction.c
@@ -333,6 +333,7 @@ void destroyfaction(faction ** fp)
 
     *fp = f->next;
     fset(f, FFL_QUIT);
+    f->_alive = false;
 
     if (f->spellbook) {
         spellbook_clear(f->spellbook);
@@ -388,7 +389,6 @@ void destroyfaction(faction ** fp)
             u = u->nextF;
         }
     }
-    f->alive = false;
     /* no way!  f->units = NULL; */
     handle_event(f->attribs, "destroy", f);
     for (ff = factions; ff; ff = ff->next) {
@@ -648,7 +648,7 @@ void remove_empty_factions(void)
     for (fp = &factions; *fp;) {
         faction *f = *fp;
 
-        if ((!f->alive || !f->units) && !fval(f, FFL_NOIDLEOUT)) {
+        if (!(f->_alive && f->units!=NULL) && !fval(f, FFL_NOIDLEOUT)) {
             log_debug("dead: %s", factionname(f));
             destroyfaction(fp);
         }
diff --git a/src/kernel/faction.test.c b/src/kernel/faction.test.c
index 6b456c9ac..9299f6370 100644
--- a/src/kernel/faction.test.c
+++ b/src/kernel/faction.test.c
@@ -76,7 +76,7 @@ static void test_remove_dead_factions(CuTest *tc) {
     CuAssertPtrEquals(tc, f, findfaction(f->no));
     CuAssertPtrNotNull(tc, get_monsters());
     fm->units = 0;
-    f->alive = 0;
+    f->_alive = false;
     fno = f->no;
     remove_empty_factions();
     CuAssertPtrEquals(tc, 0, findfaction(fno));
@@ -107,7 +107,7 @@ static void test_addfaction(CuTest *tc) {
     CuAssertIntEquals(tc, 1234, f->subscription);
     CuAssertIntEquals(tc, 0, f->flags);
     CuAssertIntEquals(tc, 0, f->age);
-    CuAssertIntEquals(tc, 1, f->alive);
+    CuAssertIntEquals(tc, true, f->_alive);
     CuAssertIntEquals(tc, M_GRAY, f->magiegebiet);
     CuAssertIntEquals(tc, turn, f->lastorders);
     CuAssertPtrEquals(tc, f, findfaction(f->no));
diff --git a/src/kernel/save.c b/src/kernel/save.c
index afcb6c988..de8f84490 100644
--- a/src/kernel/save.c
+++ b/src/kernel/save.c
@@ -1668,7 +1668,7 @@ int readgame(const char *filename, bool backup)
     log_debug("marking factions as alive.\n");
     for (f = factions; f; f = f->next) {
         if (f->flags & FFL_NPC) {
-            f->alive = 1;
+            f->_alive = true;
             f->magiegebiet = M_GRAY;
             if (f->no == 0) {
                 int no = 666;
@@ -1698,7 +1698,7 @@ int readgame(const char *filename, bool backup)
                     }
                 }
                 if (u->number > 0) {
-                    f->alive = true;
+                    f->_alive = true;
                     if (global.data_version >= SPELL_LEVEL_VERSION) {
                         break;
                     }
diff --git a/src/kernel/unit.c b/src/kernel/unit.c
index d82ad47ed..4b5c145fd 100644
--- a/src/kernel/unit.c
+++ b/src/kernel/unit.c
@@ -1463,7 +1463,7 @@ unit *create_unit(region * r, faction * f, int number, const struct race *urace,
 
     assert(urace);
     if (f) {
-        assert(f->alive);
+        assert(f->_alive);
         u_setfaction(u, f);
 
         if (f->locale) {
@@ -1828,7 +1828,7 @@ void remove_empty_units_in_region(region * r)
 
         if (u->number) {
             faction *f = u->faction;
-            if (f == NULL || !f->alive) {
+            if (f == NULL || !f->_alive) {
                 set_number(u, 0);
             }
         }
diff --git a/src/kernel/unit.test.c b/src/kernel/unit.test.c
index e72f4caed..474d839ee 100644
--- a/src/kernel/unit.test.c
+++ b/src/kernel/unit.test.c
@@ -86,7 +86,7 @@ static void test_remove_units_with_dead_faction(CuTest *tc) {
 
     u = test_create_unit(test_create_faction(test_create_race("human")), findregion(0, 0));
     uid = u->no;
-    u->faction->alive = false;
+    u->faction->_alive = false;
     remove_empty_units_in_region(u->region);
     CuAssertPtrEquals(tc, 0, findunit(uid));
     CuAssertIntEquals(tc, 0, u->number);
diff --git a/src/laws.c b/src/laws.c
index ec67c9982..904563a79 100755
--- a/src/laws.c
+++ b/src/laws.c
@@ -1244,7 +1244,7 @@ static void remove_idle_players(void)
     age = calloc(_max(4, turn + 1), sizeof(int));
     for (fp = &factions; *fp;) {
         faction *f = *fp;
-        if (f->alive && !is_monsters(f)) {
+        if (!is_monsters(f)) {
             if (RemoveNMRNewbie() && !fval(f, FFL_NOIDLEOUT)) {
                 if (f->age >= 0 && f->age <= turn)
                     ++age[f->age];
@@ -4267,18 +4267,16 @@ static int warn_password(void)
     faction *f;
     for (f = factions; f; f = f->next) {
         bool pwok = true;
-        if (f->alive) {
-            const char *c = f->passw;
-            while (*c && pwok) {
-                if (!isalnum((unsigned char)*c))
-                    pwok = false;
-                c++;
-            }
-            if (!pwok) {
-                free(f->passw);
-                f->passw = _strdup(itoa36(rng_int()));
-                ADDMSG(&f->msgs, msg_message("illegal_password", "newpass", f->passw));
-            }
+        const char *c = f->passw;
+        while (*c && pwok) {
+            if (!isalnum((unsigned char)*c))
+                pwok = false;
+            c++;
+        }
+        if (!pwok) {
+            free(f->passw);
+            f->passw = _strdup(itoa36(rng_int()));
+            ADDMSG(&f->msgs, msg_message("illegal_password", "newpass", f->passw));
         }
     }
     return 0;
diff --git a/src/summary.c b/src/summary.c
index 94ae0b6e7..093481c76 100644
--- a/src/summary.c
+++ b/src/summary.c
@@ -89,7 +89,7 @@ int update_nmrs(void)
         if (fval(f, FFL_ISNEW)) {
             ++newplayers;
         }
-        else if (!fval(f, FFL_NOIDLEOUT) && f->alive) {
+        else if (!fval(f, FFL_NOIDLEOUT)) {
             int nmr = turn - f->lastorders + 1;
             if (nmr < 0 || nmr > NMRTimeout()) {
                 log_error("faction %s has %d NMRS\n", factionid(f), nmr);
@@ -370,7 +370,7 @@ summary *make_summary(void)
         f->nregions = 0;
         f->num_total = 0;
         f->money = 0;
-        if (f->alive && f->units) {
+        if (f->units) {
             s->factions++;
             /* Problem mit Monsterpartei ... */
             if (!is_monsters(f)) {

From ca500a499e6d2a730a7305316586a495bbf585d4 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Mon, 11 Jan 2016 12:25:23 +0100
Subject: [PATCH 3/3] adding a much-needed getter function for faction_alive

---
 src/kernel/faction.c      | 5 +++++
 src/kernel/faction.h      | 2 ++
 src/kernel/faction.test.c | 2 +-
 src/kernel/unit.c         | 4 ++--
 4 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/kernel/faction.c b/src/kernel/faction.c
index 0ab13c150..fd5799265 100755
--- a/src/kernel/faction.c
+++ b/src/kernel/faction.c
@@ -658,6 +658,11 @@ void remove_empty_factions(void)
     }
 }
 
+bool faction_alive(faction *f) {
+    assert(f);
+    return f->_alive || (f->flags&FFL_NPC);
+}
+
 void faction_getorigin(const faction * f, int id, int *x, int *y)
 {
     ursprung *ur;
diff --git a/src/kernel/faction.h b/src/kernel/faction.h
index 6692636f0..722904734 100644
--- a/src/kernel/faction.h
+++ b/src/kernel/faction.h
@@ -123,6 +123,8 @@ extern "C" {
     bool checkpasswd(const faction * f, const char *passwd);
     void destroyfaction(faction ** f);
 
+    bool faction_alive(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);
 
diff --git a/src/kernel/faction.test.c b/src/kernel/faction.test.c
index 9299f6370..a94a907e8 100644
--- a/src/kernel/faction.test.c
+++ b/src/kernel/faction.test.c
@@ -107,7 +107,7 @@ static void test_addfaction(CuTest *tc) {
     CuAssertIntEquals(tc, 1234, f->subscription);
     CuAssertIntEquals(tc, 0, f->flags);
     CuAssertIntEquals(tc, 0, f->age);
-    CuAssertIntEquals(tc, true, f->_alive);
+    CuAssertIntEquals(tc, true, faction_alive(f));
     CuAssertIntEquals(tc, M_GRAY, f->magiegebiet);
     CuAssertIntEquals(tc, turn, f->lastorders);
     CuAssertPtrEquals(tc, f, findfaction(f->no));
diff --git a/src/kernel/unit.c b/src/kernel/unit.c
index 4b5c145fd..04d113033 100644
--- a/src/kernel/unit.c
+++ b/src/kernel/unit.c
@@ -1463,7 +1463,7 @@ unit *create_unit(region * r, faction * f, int number, const struct race *urace,
 
     assert(urace);
     if (f) {
-        assert(f->_alive);
+        assert(faction_alive(f));
         u_setfaction(u, f);
 
         if (f->locale) {
@@ -1828,7 +1828,7 @@ void remove_empty_units_in_region(region * r)
 
         if (u->number) {
             faction *f = u->faction;
-            if (f == NULL || !f->_alive) {
+            if (f == NULL || !faction_alive(f)) {
                 set_number(u, 0);
             }
         }