From dd2f146e274d6f2e3ce2bfaee2bfa6cb49628962 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 7 Sep 2016 17:09:37 +0100 Subject: [PATCH 01/12] fix equipment static memory leak --- src/kernel/config.c | 2 ++ src/kernel/equipment.c | 12 ++++++++++++ src/kernel/equipment.h | 1 + 3 files changed, 15 insertions(+) diff --git a/src/kernel/config.c b/src/kernel/config.c index 4758b36c8..7fae8df23 100644 --- a/src/kernel/config.c +++ b/src/kernel/config.c @@ -31,6 +31,7 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. #include "building.h" #include "calendar.h" #include "direction.h" +#include "equipment.h" #include "faction.h" #include "group.h" #include "item.h" @@ -1079,6 +1080,7 @@ void free_gamedata(void) { int i; free_donations(); + free_equipment(); for (i = 0; i != MAXLOCALES; ++i) { if (defaults[i]) { diff --git a/src/kernel/equipment.c b/src/kernel/equipment.c index 1511e2865..f5bd2fd29 100644 --- a/src/kernel/equipment.c +++ b/src/kernel/equipment.c @@ -223,3 +223,15 @@ void equip_items(struct item **items, const struct equipment *eq) } } } + +void free_equipment(void) { + equipment **eqp = &equipment_sets; + while (*eqp) { + equipment *eq = *eqp; + *eqp = eq->next; + free(eq->name); + spellbook_clear(eq->spellbook); + // TODO: items, subsets + free(eq); + } +} diff --git a/src/kernel/equipment.h b/src/kernel/equipment.h index 0bda6dc7d..1ce454111 100644 --- a/src/kernel/equipment.h +++ b/src/kernel/equipment.h @@ -75,6 +75,7 @@ extern "C" { int mask); void equip_items(struct item **items, const struct equipment *eq); + void free_equipment(void); #ifdef __cplusplus } #endif From 282cc129d738a75bbdb6e2406e107558328bba2a Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 7 Sep 2016 19:02:03 +0200 Subject: [PATCH 02/12] spellbook_clear crash fix --- src/kernel/equipment.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/kernel/equipment.c b/src/kernel/equipment.c index f5bd2fd29..7d69f26d8 100644 --- a/src/kernel/equipment.c +++ b/src/kernel/equipment.c @@ -230,7 +230,9 @@ void free_equipment(void) { equipment *eq = *eqp; *eqp = eq->next; free(eq->name); - spellbook_clear(eq->spellbook); + if (eq->spellbook) { + spellbook_clear(eq->spellbook); + } // TODO: items, subsets free(eq); } From c8283060f92bd82f9f427c77236923261966c0ce Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 7 Sep 2016 19:04:42 +0200 Subject: [PATCH 03/12] test_setup/cleanup for equipment.test --- src/kernel/equipment.test.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/kernel/equipment.test.c b/src/kernel/equipment.test.c index f0543ad8c..490a44a78 100644 --- a/src/kernel/equipment.test.c +++ b/src/kernel/equipment.test.c @@ -20,7 +20,7 @@ void test_equipment(CuTest * tc) spell *sp; sc_mage * mage; - test_cleanup(); + test_setup(); test_create_race("human"); enable_skill(SK_MAGIC, true); it_horses = test_create_itemtype("horse"); @@ -36,7 +36,7 @@ void test_equipment(CuTest * tc) equipment_setskill(eq, SK_MAGIC, "5"); equipment_addspell(eq, sp, 1); - u = test_create_unit(test_create_faction(0), 0); + u = test_create_unit(test_create_faction(0), test_create_region(0, 0, 0)); equip_unit_mask(u, eq, EQUIP_ALL); CuAssertIntEquals(tc, 1, i_get(u->items, it_horses)); CuAssertIntEquals(tc, 5, get_level(u, SK_MAGIC)); @@ -45,6 +45,7 @@ void test_equipment(CuTest * tc) CuAssertPtrNotNull(tc, mage); CuAssertPtrNotNull(tc, mage->spellbook); CuAssertTrue(tc, u_hasspell(u, sp)); + test_cleanup(); } CuSuite *get_equipment_suite(void) From dda845e2f454a0dcdb39898373cf4ce7008519e2 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 7 Sep 2016 20:46:41 +0200 Subject: [PATCH 04/12] free configuration data (valgrind calls it still-reachable leaks). --- src/eressea.c | 19 +++++++++++++------ src/kernel/config.c | 9 ++++----- src/kernel/equipment.c | 2 +- src/kernel/equipment.h | 3 ++- src/report.c | 7 ------- src/reports.c | 9 +++++++++ src/reports.h | 4 ++-- src/util/message.c | 11 ++++++++++- src/util/message.h | 4 +++- 9 files changed, 44 insertions(+), 24 deletions(-) diff --git a/src/eressea.c b/src/eressea.c index 18c98cad6..7dff11574 100755 --- a/src/eressea.c +++ b/src/eressea.c @@ -14,23 +14,27 @@ #include #include #include -#include -#include -#include #include +#include +#include +#include +#include #include #include #include #include +#include #include + #include "chaos.h" -#include "report.h" -#include "items.h" #include "creport.h" +#include "items.h" #include "jsreport.h" #include "names.h" -#include "wormhole.h" +#include "report.h" +#include "reports.h" #include "spells.h" +#include "wormhole.h" void game_done(void) { @@ -51,6 +55,9 @@ void game_done(void) free_functions(); free_config(); free_locales(); + message_done(); + equipment_done(); + reports_done(); curses_done(); kernel_done(); } diff --git a/src/kernel/config.c b/src/kernel/config.c index 7fae8df23..e45ffdde9 100644 --- a/src/kernel/config.c +++ b/src/kernel/config.c @@ -1080,7 +1080,6 @@ void free_gamedata(void) { int i; free_donations(); - free_equipment(); for (i = 0; i != MAXLOCALES; ++i) { if (defaults[i]) { @@ -1094,15 +1093,15 @@ void free_gamedata(void) free_borders(); free_alliances(); + while (global.attribs) { + a_remove(&global.attribs, global.attribs); + } + while (planes) { plane *pl = planes; planes = planes->next; free_plane(pl); } - - while (global.attribs) { - a_remove(&global.attribs, global.attribs); - } } const char * game_name(void) { diff --git a/src/kernel/equipment.c b/src/kernel/equipment.c index f5bd2fd29..15db54b32 100644 --- a/src/kernel/equipment.c +++ b/src/kernel/equipment.c @@ -224,7 +224,7 @@ void equip_items(struct item **items, const struct equipment *eq) } } -void free_equipment(void) { +void equipment_done(void) { equipment **eqp = &equipment_sets; while (*eqp) { equipment *eq = *eqp; diff --git a/src/kernel/equipment.h b/src/kernel/equipment.h index 1ce454111..11511d99a 100644 --- a/src/kernel/equipment.h +++ b/src/kernel/equipment.h @@ -54,6 +54,8 @@ extern "C" { void(*callback) (const struct equipment *, struct unit *); } equipment; + void equipment_done(void); + struct equipment *create_equipment(const char *eqname); struct equipment *get_equipment(const char *eqname); @@ -75,7 +77,6 @@ extern "C" { int mask); void equip_items(struct item **items, const struct equipment *eq); - void free_equipment(void); #ifdef __cplusplus } #endif diff --git a/src/report.c b/src/report.c index 6436fe990..294f0c7f5 100644 --- a/src/report.c +++ b/src/report.c @@ -2547,13 +2547,6 @@ static void update_find(void) initial = false; } -bool kann_finden(faction * f1, faction * f2) -{ - update_find(); - return (bool)(can_find(f1, f2) != NULL); -} - -/******* end summary ******/ void register_nr(void) { diff --git a/src/reports.c b/src/reports.c index 74ba8656a..06408d96e 100644 --- a/src/reports.c +++ b/src/reports.c @@ -1094,6 +1094,15 @@ void register_reporttype(const char *extension, report_fun write, int flag) report_types = type; } +void reports_done(void) { + report_type **rtp = &report_types; + while (*rtp) { + report_type *rt = *rtp; + *rtp = rt->next; + free(rt); + } +} + static quicklist *get_regions_distance(region * root, int radius) { quicklist *ql, *rlist = NULL; diff --git a/src/reports.h b/src/reports.h index be99fbba7..584fe7d4a 100644 --- a/src/reports.h +++ b/src/reports.h @@ -44,8 +44,8 @@ extern "C" { extern bool noreports; extern const char *visibility[]; - /* kann_finden speedups */ - bool kann_finden(struct faction *f1, struct faction *f2); + void reports_done(void); + struct unit *can_find(struct faction *, struct faction *); /* funktionen zum schreiben eines reports */ diff --git a/src/util/message.c b/src/util/message.c index 92a5b43e6..f19fc3313 100644 --- a/src/util/message.c +++ b/src/util/message.c @@ -32,7 +32,7 @@ const char *mt_name(const message_type * mtype) return mtype->name; } -arg_type *argtypes = NULL; +static arg_type *argtypes = NULL; void register_argtype(const char *name, void(*free_arg) (variant), @@ -246,3 +246,12 @@ struct message *msg_addref(struct message *msg) ++msg->refcount; return msg; } + +void message_done(void) { + arg_type **atp = &argtypes; + while (*atp) { + arg_type *at = *atp; + *atp = at->next; + free(at); + } +} diff --git a/src/util/message.h b/src/util/message.h index 687413605..5937fb83e 100644 --- a/src/util/message.h +++ b/src/util/message.h @@ -40,8 +40,10 @@ extern "C" { int refcount; } message; + void message_done(void); + void mt_clear(void); - struct message_type *mt_new(const char *name, const char **args); + struct message_type *mt_new(const char *name, const char *args[]); struct message_type *mt_new_va(const char *name, ...); /* mt_new("simple_sentence", "subject:string", "predicate:string", * "object:string", "lang:locale", NULL); */ From ef5ce043358afbb24d7784d617c2f508ed0431ee Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 7 Sep 2016 20:50:52 +0200 Subject: [PATCH 05/12] remove unused code --- src/report.c | 55 ---------------------------------------------------- 1 file changed, 55 deletions(-) diff --git a/src/report.c b/src/report.c index 294f0c7f5..e77ee3038 100644 --- a/src/report.c +++ b/src/report.c @@ -2493,61 +2493,6 @@ unit *can_find(faction * f, faction * f2) return NULL; } -static void add_find(faction * f, unit * u, faction * f2) -{ - /* faction f sees f2 through u */ - int key = f->no % FMAXHASH; - struct fsee **fp = &fsee[key]; - struct fsee *fs; - struct see **sp; - struct see *ss; - while (*fp && (*fp)->f != f) - fp = &(*fp)->nexthash; - if (!*fp) { - fs = *fp = calloc(sizeof(struct fsee), 1); - fs->f = f; - } - else - fs = *fp; - sp = &fs->see; - while (*sp && (*sp)->seen != f2) - sp = &(*sp)->next; - if (!*sp) { - ss = *sp = calloc(sizeof(struct see), 1); - ss->proof = u; - ss->seen = f2; - } - else - ss = *sp; - ss->proof = u; -} - -static void update_find(void) -{ - region *r; - static bool initial = true; - - if (initial) - for (r = regions; r; r = r->next) { - unit *u; - for (u = r->units; u; u = u->next) { - faction *lastf = u->faction; - unit *u2; - for (u2 = r->units; u2; u2 = u2->next) { - if (u2->faction == lastf || u2->faction == u->faction) - continue; - if (seefaction(u->faction, r, u2, 0)) { - faction *fv = visible_faction(u->faction, u2); - lastf = fv; - add_find(u->faction, u2, fv); - } - } - } - } - initial = false; -} - - void register_nr(void) { if (!nocr) From d84ed1f89d26559541d0a340effea0c49765ec72 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 7 Sep 2016 21:15:24 +0200 Subject: [PATCH 06/12] move static variable cleanup to kernel_done. clean up some more. --- src/eressea.c | 4 ---- src/kernel/config.c | 24 +++++++++++++++++------- src/kernel/equipment.c | 1 + src/kernel/item.c | 5 +++++ src/kernel/item.h | 2 ++ src/util/attrib.c | 3 ++- src/util/attrib.h | 2 +- src/util/crmessage.c | 9 +++++++++ src/util/crmessage.h | 2 ++ src/util/xml.c | 9 +++++++++ src/util/xml.h | 16 ++++++++++------ 11 files changed, 58 insertions(+), 19 deletions(-) diff --git a/src/eressea.c b/src/eressea.c index 7dff11574..c1ae4d73a 100755 --- a/src/eressea.c +++ b/src/eressea.c @@ -55,10 +55,6 @@ void game_done(void) free_functions(); free_config(); free_locales(); - message_done(); - equipment_done(); - reports_done(); - curses_done(); kernel_done(); } diff --git a/src/kernel/config.c b/src/kernel/config.c index e45ffdde9..89d816f0f 100644 --- a/src/kernel/config.c +++ b/src/kernel/config.c @@ -401,24 +401,24 @@ building *largestbuilding(const region * r, cmp_building_cb cmp_gt, static const char *forbidden[] = { "t", "te", "tem", "temp", NULL }; // PEASANT: "b", "ba", "bau", "baue", "p", "pe", "pea", "peas" +static int *forbidden_ids; int forbiddenid(int id) { - static int *forbid = NULL; static size_t len; size_t i; if (id <= 0) return 1; - if (!forbid) { + if (!forbidden_ids) { while (forbidden[len]) ++len; - forbid = calloc(len, sizeof(int)); + forbidden_ids = calloc(len, sizeof(int)); for (i = 0; i != len; ++i) { - forbid[i] = atoi36(forbidden[i]); + forbidden_ids[i] = atoi36(forbidden[i]); } } for (i = 0; i != len; ++i) - if (id == forbid[i]) + if (id == forbidden_ids[i]) return 1; return 0; } @@ -739,8 +739,15 @@ void kernel_done(void) /* calling this function releases memory assigned to static variables, etc. * calling it is optional, e.g. a release server will most likely not do it. */ + xml_done(); + attrib_done(); + item_done(); + message_done(); + equipment_done(); + reports_done(); + curses_done(); + crmessage_done(); translation_done(); - free_attribs(); } #ifndef HAVE_STRDUP @@ -1079,7 +1086,6 @@ void free_config(void) { void free_gamedata(void) { int i; - free_donations(); for (i = 0; i != MAXLOCALES; ++i) { if (defaults[i]) { @@ -1087,6 +1093,10 @@ void free_gamedata(void) defaults[i] = 0; } } + free(forbidden_ids); + forbidden_ids = NULL; + + free_donations(); free_factions(); free_units(); free_regions(); diff --git a/src/kernel/equipment.c b/src/kernel/equipment.c index 4afeac803..5f1c31743 100644 --- a/src/kernel/equipment.c +++ b/src/kernel/equipment.c @@ -232,6 +232,7 @@ void equipment_done(void) { free(eq->name); if (eq->spellbook) { spellbook_clear(eq->spellbook); + free(eq->spellbook); } // TODO: items, subsets free(eq); diff --git a/src/kernel/item.c b/src/kernel/item.c index 95d7461aa..66e361156 100644 --- a/src/kernel/item.c +++ b/src/kernel/item.c @@ -518,6 +518,11 @@ static item *icache; static int icache_size; #define ICACHE_MAX 100 +void item_done(void) { + i_freeall(&icache); + icache_size = 0; +} + void i_free(item * i) { if (icache_size >= ICACHE_MAX) { diff --git a/src/kernel/item.h b/src/kernel/item.h index fc3c2c909..c3b33070e 100644 --- a/src/kernel/item.h +++ b/src/kernel/item.h @@ -59,6 +59,8 @@ extern "C" { #define NMF_PLURAL 0x01 #define NMF_APPEARANCE 0x02 + void item_done(void); + typedef int(*rtype_uchange) (struct unit * user, const struct resource_type * rtype, int delta); typedef int(*rtype_uget) (const struct unit * user, diff --git a/src/util/attrib.c b/src/util/attrib.c index c22570356..e6392aeeb 100644 --- a/src/util/attrib.c +++ b/src/util/attrib.c @@ -412,6 +412,7 @@ void a_write_orig(struct storage *store, const attrib * attribs, const void *own WRITE_TOK(store, "end"); } -void free_attribs(void) { +void attrib_done(void) { cb_clear(&cb_deprecated); + memset(at_hash, 0, sizeof at_hash); } diff --git a/src/util/attrib.h b/src/util/attrib.h index b41ac2bcf..b9378ab55 100644 --- a/src/util/attrib.h +++ b/src/util/attrib.h @@ -81,7 +81,7 @@ extern "C" { int a_read(struct gamedata *data, attrib ** attribs, void *owner); void a_write(struct storage *store, const attrib * attribs, const void *owner); - void free_attribs(void); + void attrib_done(void); #define DEFAULT_AGE NULL #define DEFAULT_INIT NULL diff --git a/src/util/crmessage.c b/src/util/crmessage.c index d15228510..98afe0f05 100644 --- a/src/util/crmessage.c +++ b/src/util/crmessage.c @@ -32,6 +32,15 @@ typedef struct tsf_list { static tsf_list *tostringfs; +void crmessage_done(void) { + tsf_list **tsp = &tostringfs; + while (*tsp) { + tsf_list *ts = *tsp; + *tsp = ts->next; + free(ts); + } +} + static tostring_f tsf_find(const char *name) { if (name != NULL) { diff --git a/src/util/crmessage.h b/src/util/crmessage.h index be6b67367..3cd71d71c 100644 --- a/src/util/crmessage.h +++ b/src/util/crmessage.h @@ -22,6 +22,8 @@ extern "C" { struct message; struct message_type; + void crmessage_done(void); + typedef int(*tostring_f) (variant data, char *buffer, const void *userdata); void tsf_register(const char *name, tostring_f fun); /* registers a new type->string-function */ diff --git a/src/util/xml.c b/src/util/xml.c index 2d49f2a7d..644698119 100644 --- a/src/util/xml.c +++ b/src/util/xml.c @@ -89,6 +89,15 @@ typedef struct xml_reader { static xml_reader *xmlReaders; +void xml_done(void) { + xml_reader ** xrp = &xmlReaders; + while (*xrp) { + xml_reader *xr = *xrp; + *xrp = xr->next; + free(xr); + } +} + void xml_register_callback(xml_callback callback) { xml_reader *reader = (xml_reader *)malloc(sizeof(xml_reader)); diff --git a/src/util/xml.h b/src/util/xml.h index b5beee233..921cd3f53 100644 --- a/src/util/xml.h +++ b/src/util/xml.h @@ -21,13 +21,17 @@ extern "C" { /* new xml functions: */ #include - typedef int (*xml_callback) (xmlDocPtr); - extern void xml_register_callback(xml_callback callback); - extern double xml_fvalue(xmlNodePtr node, const char *name, double dflt); - extern int xml_ivalue(xmlNodePtr node, const char *name, int dflt); - extern bool xml_bvalue(xmlNodePtr node, const char *name, bool dflt); + + typedef int (*xml_callback) (xmlDocPtr); + + void xml_register_callback(xml_callback callback); + double xml_fvalue(xmlNodePtr node, const char *name, double dflt); + int xml_ivalue(xmlNodePtr node, const char *name, int dflt); + bool xml_bvalue(xmlNodePtr node, const char *name, bool dflt); #endif - extern int read_xml(const char *filename, const char *catalog); + + void xml_done(void); + int read_xml(const char *filename, const char *catalog); #ifdef __cplusplus } From cb706c2cf2aeca9949b718de42e17a3e97d88339 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 7 Sep 2016 21:29:54 +0200 Subject: [PATCH 07/12] equipment.test is down to two leaks and one TODO --- src/kernel/equipment.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/kernel/equipment.c b/src/kernel/equipment.c index 5f1c31743..2debc9aa9 100644 --- a/src/kernel/equipment.c +++ b/src/kernel/equipment.c @@ -234,7 +234,13 @@ void equipment_done(void) { spellbook_clear(eq->spellbook); free(eq->spellbook); } - // TODO: items, subsets + while (eq->items) { + itemdata *next = eq->items->next; + free(eq->items->value); + free(eq->items); + eq->items = next; + } + // TODO: subsets free(eq); } } From 993af3a7f973670a6cdb2a00fa144030d02a4cf8 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Thu, 8 Sep 2016 06:56:16 +0200 Subject: [PATCH 08/12] memory leak in equipment.skills --- src/kernel/equipment.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/kernel/equipment.c b/src/kernel/equipment.c index 2debc9aa9..424352347 100644 --- a/src/kernel/equipment.c +++ b/src/kernel/equipment.c @@ -227,6 +227,7 @@ void equip_items(struct item **items, const struct equipment *eq) void equipment_done(void) { equipment **eqp = &equipment_sets; while (*eqp) { + int i; equipment *eq = *eqp; *eqp = eq->next; free(eq->name); @@ -240,7 +241,10 @@ void equipment_done(void) { free(eq->items); eq->items = next; } - // TODO: subsets + // TODO: subsets, skills + for (i=0;i!=MAXSKILLS;++i) { + free(eq->skills[i]); + } free(eq); } } From 25e5d1d2857549b470c9aefaf0ed38c29fa714b5 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Thu, 8 Sep 2016 08:12:26 +0200 Subject: [PATCH 09/12] use test_setup to start tests --- src/magic.test.c | 26 +++++++++++++------------- src/tests.test.c | 4 ++-- src/volcano.test.c | 4 ++-- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/magic.test.c b/src/magic.test.c index ff6cb1e29..4ba867b96 100644 --- a/src/magic.test.c +++ b/src/magic.test.c @@ -27,7 +27,7 @@ void test_updatespells(CuTest * tc) spell * sp; spellbook *book = 0; - test_cleanup(); + test_setup(); test_create_race("human"); f = test_create_faction(0); @@ -53,7 +53,7 @@ void test_spellbooks(CuTest * tc) spellbook *herp, *derp; spellbook_entry *entry; const char * sname = "herpderp"; - test_cleanup(); + test_setup(); herp = get_spellbook("herp"); CuAssertPtrNotNull(tc, herp); @@ -85,7 +85,7 @@ void test_pay_spell(CuTest * tc) region * r; int level; - test_cleanup(); + test_setup(); test_create_world(); r = findregion(0, 0); f = test_create_faction(0); @@ -119,7 +119,7 @@ void test_pay_spell_failure(CuTest * tc) struct region * r; int level; - test_cleanup(); + test_setup(); test_create_world(); r = findregion(0, 0); f = test_create_faction(0); @@ -157,7 +157,7 @@ void test_getspell_unit(CuTest * tc) struct region * r; struct locale * lang; - test_cleanup(); + test_setup(); test_create_world(); r = findregion(0, 0); f = test_create_faction(0); @@ -186,7 +186,7 @@ void test_getspell_faction(CuTest * tc) struct region * r; struct locale * lang; - test_cleanup(); + test_setup(); test_create_world(); r = findregion(0, 0); f = test_create_faction(0); @@ -218,7 +218,7 @@ void test_getspell_school(CuTest * tc) struct locale * lang; struct spellbook * book; - test_cleanup(); + test_setup(); test_create_world(); r = findregion(0, 0); f = test_create_faction(0); @@ -249,7 +249,7 @@ void test_set_pre_combatspell(CuTest * tc) struct region * r; const int index = 0; - test_cleanup(); + test_setup(); test_create_world(); r = findregion(0, 0); f = test_create_faction(0); @@ -282,7 +282,7 @@ void test_set_main_combatspell(CuTest * tc) struct region * r; const int index = 1; - test_cleanup(); + test_setup(); test_create_world(); r = findregion(0, 0); f = test_create_faction(0); @@ -315,7 +315,7 @@ void test_set_post_combatspell(CuTest * tc) struct region * r; const int index = 2; - test_cleanup(); + test_setup(); test_create_world(); r = findregion(0, 0); f = test_create_faction(0); @@ -347,7 +347,7 @@ void test_hasspell(CuTest * tc) struct faction * f; struct region * r; - test_cleanup(); + test_setup(); test_create_world(); r = findregion(0, 0); f = test_create_faction(0); @@ -382,7 +382,7 @@ void test_multi_cast(CuTest *tc) { spell *sp; struct locale * lang; - test_cleanup(); + test_setup(); sp = create_spell("fireball", 0); sp->cast = cast_fireball; CuAssertPtrEquals(tc, sp, find_spell("fireball")); @@ -410,7 +410,7 @@ static void test_magic_resistance(CuTest *tc) { unit *u; race *rc; - test_cleanup(); + test_setup(); rc = test_create_race("human"); u = test_create_unit(test_create_faction(rc), test_create_region(0, 0, 0)); CuAssertDblEquals(tc, rc->magres, magic_resistance(u), 0.01); diff --git a/src/tests.test.c b/src/tests.test.c index 3a9732609..9dd47e4d1 100644 --- a/src/tests.test.c +++ b/src/tests.test.c @@ -12,7 +12,7 @@ static void test_resources(CuTest *tc) { resource_type *rtype; - test_cleanup(); + test_setup(); init_resources(); CuAssertPtrNotNull(tc, rt_find("hp")); CuAssertPtrEquals(tc, rt_find("hp"), (void *)get_resourcetype(R_LIFE)); @@ -36,7 +36,7 @@ static void test_resources(CuTest *tc) { static void test_recreate_world(CuTest * tc) { - test_cleanup(); + test_setup(); CuAssertPtrEquals(tc, 0, get_locale("de")); CuAssertPtrEquals(tc, 0, (void *)rt_find("horse")); diff --git a/src/volcano.test.c b/src/volcano.test.c index 091615fed..3afa49651 100644 --- a/src/volcano.test.c +++ b/src/volcano.test.c @@ -19,7 +19,7 @@ static void test_volcano_update(CuTest *tc) { message *m; const struct terrain_type *t_volcano, *t_active; - test_cleanup(); + test_setup(); mt_register(mt_new_va("volcanostopsmoke", "region:region", 0)); t_volcano = test_create_terrain("volcano", LAND_REGION); t_active = test_create_terrain("activevolcano", LAND_REGION); @@ -41,7 +41,7 @@ static void test_volcano_outbreak(CuTest *tc) { message *m; const struct terrain_type *t_volcano, *t_active; - test_cleanup(); + test_setup(); mt_register(mt_new_va("volcanooutbreak", "regionv:region", "regionn:region", 0)); mt_register(mt_new_va("volcano_dead", "unit:unit", "region:region", "dead:int", 0)); t_volcano = test_create_terrain("volcano", LAND_REGION); From 22586aa45fddb780784660979b16f54efea38944 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Thu, 8 Sep 2016 09:06:02 +0200 Subject: [PATCH 10/12] fix seen.test creating a unit without a region --- src/kernel/save.test.c | 2 +- src/seen.test.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/kernel/save.test.c b/src/kernel/save.test.c index e88cf7b2b..12ab11cc3 100644 --- a/src/kernel/save.test.c +++ b/src/kernel/save.test.c @@ -155,10 +155,10 @@ static void test_readwrite_dead_faction_regionowner(CuTest *tc) { gamedata data; storage store; + test_setup(); mstream_init(&data.strm); gamedata_init(&data, &store, RELEASE_VERSION); - test_cleanup(); config_set("rules.region_owners", "1"); f = test_create_faction(0); test_create_unit(f, r = test_create_region(0, 0, 0)); diff --git a/src/seen.test.c b/src/seen.test.c index 66a476135..e3e1265bd 100644 --- a/src/seen.test.c +++ b/src/seen.test.c @@ -92,9 +92,9 @@ static void test_seen_travelthru(CuTest *tc) { test_setup(); setup_seen(0, 0); - r = test_create_region(0, 0, 0); f = test_create_faction(0); - u = test_create_unit(f, 0); + u = test_create_unit(f, test_create_region(0, 0, 0)); + r = test_create_region(0, 1, 0); travelthru_add(r, u); init_reports(); view_default(f->seen, r, f); @@ -190,10 +190,10 @@ static void test_seenhash_map(CuTest *tc) { CuSuite *get_seen_suite(void) { CuSuite *suite = CuSuiteNew(); - SUITE_ADD_TEST(suite, test_add_seen); - SUITE_ADD_TEST(suite, test_faction_add_seen); SUITE_ADD_TEST(suite, test_prepare_seen); SUITE_ADD_TEST(suite, test_seen_travelthru); + SUITE_ADD_TEST(suite, test_add_seen); + SUITE_ADD_TEST(suite, test_faction_add_seen); SUITE_ADD_TEST(suite, test_seen_region); SUITE_ADD_TEST(suite, test_seen_interval_backward); SUITE_ADD_TEST(suite, test_seen_interval_forward); From a281ace21b31de1addce77b5faeb185c2cc29227 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Thu, 8 Sep 2016 09:11:17 +0200 Subject: [PATCH 11/12] always create test units inside a region (so they can get destroyed in free_gamedata) --- src/kernel/item.test.c | 2 +- src/reports.test.c | 2 +- src/study.test.c | 2 +- src/tests.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/kernel/item.test.c b/src/kernel/item.test.c index 47b32b537..231b45d00 100644 --- a/src/kernel/item.test.c +++ b/src/kernel/item.test.c @@ -74,7 +74,7 @@ void test_change_item(CuTest * tc) test_create_itemtype("iron"); init_resources(); - u = test_create_unit(test_create_faction(0), 0); + u = test_create_unit(test_create_faction(0), test_create_region(0, 0, 0)); test_uchange(tc, u, get_resourcetype(R_IRON)); test_cleanup(); } diff --git a/src/reports.test.c b/src/reports.test.c index f368e63e1..c27a0f6f9 100644 --- a/src/reports.test.c +++ b/src/reports.test.c @@ -220,7 +220,7 @@ static void test_write_travelthru(CuTest *tc) { r->flags |= RF_TRAVELUNIT; f = test_create_faction(0); f->locale = lang; - u = test_create_unit(f, 0); + u = test_create_unit(f, test_create_region(0, 1, 0)); unit_setname(u, "Hodor"); unit_setid(u, 1); diff --git a/src/study.test.c b/src/study.test.c index f12fc25df..0ecd7ae2e 100644 --- a/src/study.test.c +++ b/src/study.test.c @@ -295,7 +295,7 @@ static void test_demon_skillchanges(CuTest *tc) { test_setup(); rc = test_create_race("demon"); CuAssertPtrEquals(tc, rc, get_race(RC_DAEMON)); - u = test_create_unit(test_create_faction(rc), 0); + u = test_create_unit(test_create_faction(rc), test_create_region(0, 0, 0)); CuAssertPtrNotNull(tc, u); set_level(u, SK_CROSSBOW, 1); demon_skillchange(u); diff --git a/src/tests.c b/src/tests.c index 13400ebd4..b9336bb25 100644 --- a/src/tests.c +++ b/src/tests.c @@ -119,7 +119,7 @@ struct faction *test_create_faction(const struct race *rc) struct unit *test_create_unit(struct faction *f, struct region *r) { const struct race * rc = f ? f->race : 0; - assert(f || !r); + assert(f && r); if (!rc) rc = rc_get_or_create("human"); return create_unit(r, f, 1, rc ? rc : rc_get_or_create("human"), 0, 0, 0); } From ab876431de082b989fb3885729cff9090633a4b3 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Thu, 8 Sep 2016 19:48:36 +0200 Subject: [PATCH 12/12] reset turn to 0 before and after each test. save tests that depend on the global turn variable would fail when being run out of order. this cause issue #560 --- src/kernel/save.c | 14 ++++++++------ src/kernel/save.test.c | 2 +- src/tests.c | 1 + 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/kernel/save.c b/src/kernel/save.c index fb0c0eba4..fccd8468e 100644 --- a/src/kernel/save.c +++ b/src/kernel/save.c @@ -623,12 +623,14 @@ static void write_owner(struct gamedata *data, region_owner * owner) if (owner) { faction *f; WRITE_INT(data->store, owner->since_turn); - WRITE_INT(data->store, owner->morale_turn); - WRITE_INT(data->store, owner->flags); - f = owner->last_owner; - write_faction_reference((f && f->_alive) ? f : NULL, data->store); - f = owner->owner; - write_faction_reference((f && f->_alive) ? f : NULL, data->store); + if (owner->since_turn >= 0) { + WRITE_INT(data->store, owner->morale_turn); + WRITE_INT(data->store, owner->flags); + f = owner->last_owner; + write_faction_reference((f && f->_alive) ? f : NULL, data->store); + f = owner->owner; + write_faction_reference((f && f->_alive) ? f : NULL, data->store); + } } else { WRITE_INT(data->store, -1); diff --git a/src/kernel/save.test.c b/src/kernel/save.test.c index 12ab11cc3..9ab6c4d50 100644 --- a/src/kernel/save.test.c +++ b/src/kernel/save.test.c @@ -162,7 +162,7 @@ static void test_readwrite_dead_faction_regionowner(CuTest *tc) { config_set("rules.region_owners", "1"); f = test_create_faction(0); test_create_unit(f, r = test_create_region(0, 0, 0)); - region_set_owner(r, f, turn); + region_set_owner(r, f, 0); destroyfaction(&factions); CuAssertTrue(tc, !f->_alive); remove_empty_units(); diff --git a/src/tests.c b/src/tests.c index b9336bb25..1af387543 100644 --- a/src/tests.c +++ b/src/tests.c @@ -161,6 +161,7 @@ void test_log_stderr(int flags) { static void test_reset(void) { int i; + turn = 0; default_locale = 0; free_gamedata(); free_terrains();