From 04624179cea458989fad7eca80953ac9b59d74e3 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Mon, 11 Jan 2016 14:42:36 +0100 Subject: [PATCH 1/5] make destroyfaction not have to loop over all factions to clear HELP status. --- src/kernel/ally.c | 3 +++ src/kernel/faction.c | 12 +++++++++--- src/kernel/faction.h | 6 +++--- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/kernel/ally.c b/src/kernel/ally.c index d53973328..611db8d7b 100644 --- a/src/kernel/ally.c +++ b/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) { diff --git a/src/kernel/faction.c b/src/kernel/faction.c index 87a8e2203..e5f18a18e 100755 --- a/src/kernel/faction.c +++ b/src/kernel/faction.c @@ -74,6 +74,9 @@ faction *factions; 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); @@ -389,8 +392,9 @@ void destroyfaction(faction ** fp) u = u->nextF; } } - /* no way! f->units = NULL; */ + handle_event(f->attribs, "destroy", f); +/* alliedgroup and others should check sf.faction.alive before using a faction from f.allies for (ff = factions; ff; ff = ff->next) { group *g; ally *sf, **sfp; @@ -417,7 +421,7 @@ void destroyfaction(faction ** fp) } } } - +*/ if (f->alliance && f->alliance->_leader == f) { setalliance(f, 0); } @@ -650,6 +654,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 +663,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); } diff --git a/src/kernel/faction.h b/src/kernel/faction.h index 722904734..6c76f5383 100644 --- a/src/kernel/faction.h +++ b/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); From 50eba2d308ed070e453e70bfd411020a63d81b6d Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Mon, 11 Jan 2016 14:59:25 +0100 Subject: [PATCH 2/5] I don't really believe that this is going to work, but let's see where it fails. --- src/kernel/faction.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/kernel/faction.c b/src/kernel/faction.c index e5f18a18e..ec80160cc 100755 --- a/src/kernel/faction.c +++ b/src/kernel/faction.c @@ -332,7 +332,6 @@ void destroyfaction(faction ** fp) { faction * f = *fp; unit *u = f->units; - faction *ff; *fp = f->next; fset(f, FFL_QUIT); @@ -394,7 +393,8 @@ void destroyfaction(faction ** fp) } handle_event(f->attribs, "destroy", f); -/* alliedgroup and others should check sf.faction.alive before using a faction from f.allies +#if 0 + faction *ff; for (ff = factions; ff; ff = ff->next) { group *g; ally *sf, **sfp; @@ -421,7 +421,8 @@ void destroyfaction(faction ** fp) } } } -*/ +#endif + if (f->alliance && f->alliance->_leader == f) { setalliance(f, 0); } @@ -431,6 +432,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) { From 04e2fb73c78230ba6431ebb52cdae219e05475e3 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Mon, 11 Jan 2016 15:08:36 +0100 Subject: [PATCH 3/5] fix test of allies status after destroying a faction --- src/kernel/faction.test.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/kernel/faction.test.c b/src/kernel/faction.test.c index a94a907e8..9c85884b7 100644 --- a/src/kernel/faction.test.c +++ b/src/kernel/faction.test.c @@ -16,18 +16,23 @@ #include #include -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); From 20063e0e0e6cd69a9bc0459db09fb5f3d4c9f4ef Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Mon, 11 Jan 2016 18:03:53 +0100 Subject: [PATCH 4/5] fix errors in the order of cleanup --- src/kernel/config.c | 9 ++++----- src/tests.c | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/kernel/config.c b/src/kernel/config.c index 56022ef74..8d98165ca 100644 --- a/src/kernel/config.c +++ b/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,16 @@ void free_gamedata(void) defaults[i] = 0; } } - free_alliances(); while (factions) { faction *f = factions; factions = f->next; - funhash(f); free_faction(f); free(f); } + free_units(); + free_regions(); + free_borders(); + free_alliances(); while (planes) { plane *pl = planes; diff --git a/src/tests.c b/src/tests.c index eb03820a8..c5f98502e 100644 --- a/src/tests.c +++ b/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(); From fadc92ee522db5501e99f2781e5508448bc905e7 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Mon, 11 Jan 2016 18:17:24 +0100 Subject: [PATCH 5/5] do not leak memory for factions that have died. --- src/kernel/config.c | 7 +------ src/kernel/faction.c | 23 ++++++++++++++++++++++- src/kernel/faction.h | 2 +- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/kernel/config.c b/src/kernel/config.c index 8d98165ca..bd9a30d81 100644 --- a/src/kernel/config.c +++ b/src/kernel/config.c @@ -1065,12 +1065,7 @@ void free_gamedata(void) defaults[i] = 0; } } - while (factions) { - faction *f = factions; - factions = f->next; - free_faction(f); - free(f); - } + free_factions(); free_units(); free_regions(); free_borders(); diff --git a/src/kernel/faction.c b/src/kernel/faction.c index ec80160cc..d13c1ea90 100755 --- a/src/kernel/faction.c +++ b/src/kernel/faction.c @@ -71,7 +71,7 @@ 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) { @@ -328,12 +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; *fp = f->next; + f->next = dead_factions; + dead_factions = f; + fset(f, FFL_QUIT); f->_alive = false; diff --git a/src/kernel/faction.h b/src/kernel/faction.h index 6c76f5383..933421da5 100644 --- a/src/kernel/faction.h +++ b/src/kernel/faction.h @@ -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