Merge pull request #454 from ennorehling/feature/issue-451-destroyfaction-cleanup

destroyfaction cleanup

src/kernel/ally.c

@@ -170,6 +170,9 @@ int
 alliedgroup(const struct plane *pl, const struct faction *f,
     const struct faction *f2, const struct ally *sf, int mode)
 {
+    if (!(faction_alive(f) && faction_alive(f2))) {
+        return 0;
+    }
     while (sf && sf->faction != f2)
         sf = sf->next;
     if (sf == NULL) {

src/kernel/config.c

@@ -1058,9 +1058,6 @@ void free_gamedata(void)
 {
     int i;
     free_donations();
-    free_units();
-    free_regions();
-    free_borders();
 
     for (i = 0; i != MAXLOCALES; ++i) {
         if (defaults[i]) {
@@ -1068,14 +1065,11 @@ void free_gamedata(void)
             defaults[i] = 0;
         }
     }
+    free_factions();
+    free_units();
+    free_regions();
+    free_borders();
     free_alliances();
-    while (factions) {
-        faction *f = factions;
-        factions = f->next;
-        funhash(f);
-        free_faction(f);
-        free(f);
-    }
 
     while (planes) {
         plane *pl = planes;

src/kernel/faction.c

@@ -71,9 +71,12 @@ faction *factions;
  * but you should still call funhash and remove the faction from the
  * global list.
  */
-void free_faction(faction * f)
+static void free_faction(faction * f)
 {
     funhash(f);
+    if (f->alliance && f->alliance->_leader == f) {
+        setalliance(f, 0);
+    }
     if (f->msgs) {
         free_messagelist(f->msgs->begin);
         free(f->msgs);
@@ -325,13 +328,33 @@ void write_faction_reference(const faction * f, struct storage *store)
     WRITE_INT(store, (f && f->_alive) ? f->no : 0);
 }
 
+static faction *dead_factions;
+
+void free_flist(faction **fp) {
+    faction * flist = *fp;
+    for (flist = factions; flist;) {
+        faction *f = flist;
+        flist = f->next;
+        free_faction(f);
+        free(f);
+    }
+    *fp = 0;
+}
+
+void free_factions(void) {
+    free_flist(&factions);
+    free_flist(&dead_factions);
+}
+
 void destroyfaction(faction ** fp)
 {
     faction * f = *fp;
     unit *u = f->units;
-    faction *ff;
 
     *fp = f->next;
+    f->next = dead_factions;
+    dead_factions = f;
+
     fset(f, FFL_QUIT);
     f->_alive = false;
 
@@ -389,8 +412,10 @@ void destroyfaction(faction ** fp)
             u = u->nextF;
         }
     }
-    /* no way!  f->units = NULL; */
+
     handle_event(f->attribs, "destroy", f);
+#if 0
+    faction *ff;
     for (ff = factions; ff; ff = ff->next) {
         group *g;
         ally *sf, **sfp;
@@ -417,6 +442,7 @@ void destroyfaction(faction ** fp)
             }
         }
     }
+#endif
 
     if (f->alliance && f->alliance->_leader == f) {
         setalliance(f, 0);
@@ -427,6 +453,7 @@ void destroyfaction(faction ** fp)
     /* units of other factions that were disguised as this faction
      * have their disguise replaced by ordinary faction hiding. */
     if (rule_stealth_other()) {
+        // TODO: f.alive should be tested for in get_otherfaction
         region *rc;
         for (rc = regions; rc; rc = rc->next) {
             for (u = rc->units; u; u = u->next) {
@@ -650,6 +677,8 @@ void remove_empty_factions(void)
         if (!(f->_alive && f->units!=NULL) && !fval(f, FFL_NOIDLEOUT)) {
             log_debug("dead: %s", factionname(f));
             destroyfaction(fp);
+            free_faction(f);
+            free(f);
         }
         else {
             fp = &(*fp)->next;
@@ -657,7 +686,7 @@ void remove_empty_factions(void)
     }
 }
 
-bool faction_alive(faction *f) {
+bool faction_alive(const faction *f) {
     assert(f);
     return f->_alive || (f->flags&FFL_NPC);
 }

src/kernel/faction.h

@@ -82,8 +82,8 @@ extern "C" {
         int num_total;              /* Anzahl Personen mit Monstern */
         int options;
         int no_units;
-        struct ally *allies;
-        struct group *groups;
+        struct ally *allies; /* alliedgroup and others should check sf.faction.alive before using a faction from f.allies */
+        struct group *groups; /* alliedgroup and others should check sf.faction.alive before using a faction from f.groups */
         int nregions;
         int money;
         score_t score;
@@ -123,7 +123,7 @@ extern "C" {
     bool checkpasswd(const faction * f, const char *passwd);
     void destroyfaction(faction ** f);
 
-    bool faction_alive(struct faction *f);
+    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);
@@ -136,7 +136,7 @@ extern "C" {
     int resolve_faction(variant data, void *addr);
 
     void renumber_faction(faction * f, int no);
-    void free_faction(struct faction *f);
+    void free_factions(void);
     void remove_empty_factions(void);
 
 #ifdef SMART_INTERVALS

src/kernel/faction.test.c

@@ -16,18 +16,23 @@
 #include <assert.h>
 #include <stdio.h>
 
-static void test_remove_empty_factions_allies(CuTest *tc) {
+static void test_destroyfaction_allies(CuTest *tc) {
     faction *f1, *f2;
     region *r;
+    ally *al;
 
     test_cleanup();
     r = test_create_region(0, 0, 0);
     f1 = test_create_faction(0);
     test_create_unit(f1, r);
     f2 = test_create_faction(0);
-    ally_add(&f1->allies, f2);
-    remove_empty_factions();
-    CuAssertPtrEquals(tc, 0, f1->allies);
+    al = ally_add(&f1->allies, f2);
+    al->status = HELP_FIGHT;
+    CuAssertIntEquals(tc, HELP_FIGHT, alliedgroup(0, f1, f2, f1->allies, 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));
     test_cleanup();
 }
 
@@ -174,7 +179,7 @@ CuSuite *get_faction_suite(void)
     CuSuite *suite = CuSuiteNew();
     SUITE_ADD_TEST(suite, test_addfaction);
     SUITE_ADD_TEST(suite, test_remove_empty_factions);
-    SUITE_ADD_TEST(suite, test_remove_empty_factions_allies);
+    SUITE_ADD_TEST(suite, test_destroyfaction_allies);
     SUITE_ADD_TEST(suite, test_remove_empty_factions_alliance);
     SUITE_ADD_TEST(suite, test_remove_dead_factions);
     SUITE_ADD_TEST(suite, test_get_monsters);

src/tests.c

@@ -78,6 +78,7 @@ void test_cleanup(void)
 {
     int i;
 
+    free_gamedata();
     free_terrains();
     free_resources();
     free_config();
@@ -89,7 +90,6 @@ void test_cleanup(void)
     free_shiptypes();
     free_races();
     free_spellbooks();
-    free_gamedata();
     free_seen();
     free_prefixes();
     mt_clear();