From ae41cafcb49a6b0c8d6eb8cda1345d650d4b557d Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sun, 12 Mar 2017 11:19:40 +0100 Subject: [PATCH 1/8] fix double free in new lighthouse code --- CHANGELOG.md | 16 ++++++++++++++++ src/reports.c | 1 - 2 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 000000000..16ec084e0 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,16 @@ +# Geplante Aenderungen in zukuenftigen Eressea-Versionen + +Als Anhaltspunkt fuer die Empfaenger der woechentlichen Testauswertungnen +will ich versuchen, die Liste meiner Aenderungen seit dem letzten Release +zu dokumentieren. + +## Version 3.12.0 + +- [other] optimierte Berechnung der Sichtbarkeit von Leuchttuermen +- [bug] Einheitenlimit bei GIB PERSON beachten +- [bug] Einheitenlimit bei ALLIANCE JOIN beachten +- [rule] Einheiten- und Personenzahl im Report beinhaltet *alle* Einheiten der Partei. +- [other] Statistik fuer Spielleiter zeigt das Parteisilber nicht mehr an. +- [other] Berechnung der Message-Ids optimiert. + + diff --git a/src/reports.c b/src/reports.c index 1e4e43061..88f6ebb21 100644 --- a/src/reports.c +++ b/src/reports.c @@ -1242,7 +1242,6 @@ static void prepare_lighthouse_ql(faction *f, selist *rlist) { add_seen_nb(f, rl, seen_lighthouse); } } - selist_free(rlist); } static void prepare_lighthouse(faction *f, region *r, int range) From 39ec03b2af3995cd76efab8db16a58b15061c129 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sun, 12 Mar 2017 12:02:29 +0100 Subject: [PATCH 2/8] coverity CID 164473 remove logically dead code. --- src/spy.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/spy.c b/src/spy.c index c9667f7ee..08662ea8e 100644 --- a/src/spy.c +++ b/src/spy.c @@ -199,8 +199,6 @@ static bool can_set_factionstealth(const unit * u, const faction * f) } ru = ru->next; } - if (ru != NULL) - break; } mu = mu->nextF; } From 720c41c1bf35be39be6c6bf6539d03ed194a5c66 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sun, 12 Mar 2017 12:53:10 +0100 Subject: [PATCH 3/8] reproduce bug 2313. recruiting breaks f->num_units. --- src/economy.test.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/economy.test.c b/src/economy.test.c index 7163da7e1..1fca4d194 100644 --- a/src/economy.test.c +++ b/src/economy.test.c @@ -316,8 +316,12 @@ static void test_recruit(CuTest *tc) { f = test_create_faction(0); u = test_create_unit(f, test_create_region(0, 0, 0)); CuAssertIntEquals(tc, 1, u->number); + CuAssertIntEquals(tc, 1, f->num_people); + CuAssertIntEquals(tc, 1, f->num_units); add_recruits(u, 1, 1); CuAssertIntEquals(tc, 2, u->number); + CuAssertIntEquals(tc, 2, f->num_people); + CuAssertIntEquals(tc, 1, f->num_units); CuAssertPtrEquals(tc, u, f->units); CuAssertPtrEquals(tc, NULL, u->nextF); CuAssertPtrEquals(tc, NULL, u->prevF); From decc38a05688fcadebcda2370fc41cac79419fa8 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sun, 12 Mar 2017 13:02:55 +0100 Subject: [PATCH 4/8] BUG 2313: proper accounting in remove_unit. https://bugs.eressea.de/view.php?id=2313 --- src/kernel/unit.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/kernel/unit.c b/src/kernel/unit.c index 2239cf81b..59629bc61 100644 --- a/src/kernel/unit.c +++ b/src/kernel/unit.c @@ -438,8 +438,11 @@ int remove_unit(unit ** ulist, unit * u) *ulist = u->next; } - if (u->faction && u->faction->units == u) { - u->faction->units = u->nextF; + if (u->faction) { + --u->faction->num_units; + if (u->faction->units == u) { + u->faction->units = u->nextF; + } } if (u->prevF) { u->prevF->nextF = u->nextF; From d8e5feac61b36192fb3bc0ae377b09013422f793 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sun, 12 Mar 2017 13:23:40 +0100 Subject: [PATCH 5/8] additional test for bug 2313. do not count spells toward num_unit or num_people. --- src/kernel/unit.test.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/kernel/unit.test.c b/src/kernel/unit.test.c index e1654b821..3932bb239 100644 --- a/src/kernel/unit.test.c +++ b/src/kernel/unit.test.c @@ -519,6 +519,39 @@ static void test_heal_factor(CuTest *tc) { test_cleanup(); } +static void test_unlimited_units(CuTest *tc) { + race *rc; + faction *f; + unit *u; + + test_setup(); + f = test_create_faction(NULL); + rc = test_create_race("spell"); + rc->flags |= RCF_INVISIBLE; + CuAssertIntEquals(tc, 0, f->num_units); + CuAssertIntEquals(tc, 0, f->num_people); + u = test_create_unit(f, test_create_region(0, 0, NULL)); +// CuAssertTrue(tc, count_unit(u)); + CuAssertIntEquals(tc, 1, f->num_units); + CuAssertIntEquals(tc, 1, f->num_people); + u_setfaction(u, NULL); + CuAssertIntEquals(tc, 0, f->num_units); + CuAssertIntEquals(tc, 0, f->num_people); + u_setfaction(u, f); + CuAssertIntEquals(tc, 1, f->num_units); + CuAssertIntEquals(tc, 1, f->num_people); + u_setrace(u, rc); + CuAssertIntEquals(tc, 0, f->num_units); + CuAssertIntEquals(tc, 0, f->num_people); + scale_number(u, 10); + CuAssertIntEquals(tc, 0, f->num_units); + CuAssertIntEquals(tc, 0, f->num_people); + u_setrace(u, f->race); + CuAssertIntEquals(tc, 1, f->num_units); + CuAssertIntEquals(tc, 10, f->num_people); + test_cleanup(); +} + CuSuite *get_unit_suite(void) { CuSuite *suite = CuSuiteNew(); @@ -534,6 +567,7 @@ CuSuite *get_unit_suite(void) SUITE_ADD_TEST(suite, test_remove_units_with_dead_faction); SUITE_ADD_TEST(suite, test_remove_empty_units_in_region); SUITE_ADD_TEST(suite, test_names); + SUITE_ADD_TEST(suite, test_unlimited_units); SUITE_ADD_TEST(suite, test_default_name); SUITE_ADD_TEST(suite, test_skillmod); SUITE_ADD_TEST(suite, test_skill_hunger); From a93dc5459ba7f75f887b206af56d6089db990f76 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sun, 12 Mar 2017 13:39:25 +0100 Subject: [PATCH 6/8] BUG 2313: spells must not be counted. https://bugs.eressea.de/view.php?id=2313 --- src/kernel/unit.c | 50 +++++++++++++++++++++++++++++++----------- src/kernel/unit.h | 1 + src/kernel/unit.test.c | 6 ++++- 3 files changed, 43 insertions(+), 14 deletions(-) diff --git a/src/kernel/unit.c b/src/kernel/unit.c index 59629bc61..ddc8e0e4a 100644 --- a/src/kernel/unit.c +++ b/src/kernel/unit.c @@ -439,7 +439,9 @@ int remove_unit(unit ** ulist, unit * u) } if (u->faction) { - --u->faction->num_units; + if (count_unit(u)) { + --u->faction->num_units; + } if (u->faction->units == u) { u->faction->units = u->nextF; } @@ -1128,12 +1130,13 @@ struct building *inside_building(const struct unit *u) void u_setfaction(unit * u, faction * f) { - int cnt = u->number; if (u->faction == f) return; if (u->faction) { - --u->faction->num_units; - set_number(u, 0); + if (count_unit(u)) { + --u->faction->num_units; + u->faction->num_people -= u->number; + } join_group(u, NULL); free_orders(&u->orders); set_order(&u->thisorder, NULL); @@ -1165,14 +1168,19 @@ void u_setfaction(unit * u, faction * f) if (u->region) { update_interval(f, u->region); } - if (cnt) { - set_number(u, cnt); - } - if (f) { + if (f && count_unit(u)) { ++f->num_units; + f->num_people += u->number; } } +bool count_unit(const unit *u) +{ + const race *rc = u_race(u); + /* spells are invisible. units we cannot see do not count to our limit */ + return rc == NULL || (rc->flags & RCF_INVISIBLE) == 0; +} + void set_number(unit * u, int count) { assert(count >= 0); @@ -1181,10 +1189,10 @@ void set_number(unit * u, int count) if (count == 0) { u->flags &= ~(UFL_HERO); } - if (u->faction) { + if (u->faction && count_unit(u)) { u->faction->num_people += count - u->number; } - u->number = (unsigned short)count; + u->number = count; } void remove_skill(unit * u, skill_t sk) @@ -1487,6 +1495,8 @@ unit *create_unit(region * r, faction * f, int number, const struct race *urace, unit *u = (unit *)calloc(1, sizeof(unit)); assert(urace); + u_setrace(u, urace); + u->irace = NULL; if (f) { assert(faction_alive(f)); u_setfaction(u, f); @@ -1499,8 +1509,6 @@ unit *create_unit(region * r, faction * f, int number, const struct race *urace, } } } - u_setrace(u, urace); - u->irace = NULL; set_number(u, number); @@ -1806,7 +1814,23 @@ const struct race *u_race(const struct unit *u) void u_setrace(struct unit *u, const struct race *rc) { assert(rc); - u->_race = rc; + if (!u->faction) { + u->_race = rc; + } + else { + int n = 0; + if (count_unit(u)) { + --n; + } + u->_race = rc; + if (count_unit(u)) { + ++n; + } + if (n != 0) { + u->faction->num_units += n; + u->faction->num_people += n * u->number; + } + } } void unit_add_spell(unit * u, sc_mage * m, struct spell * sp, int level) diff --git a/src/kernel/unit.h b/src/kernel/unit.h index 14096c872..8ae13c5e2 100644 --- a/src/kernel/unit.h +++ b/src/kernel/unit.h @@ -130,6 +130,7 @@ extern "C" { int weight(const struct unit *u); void renumber_unit(struct unit *u, int no); + bool count_unit(const unit *u); /* unit counts towards faction.num_units and faction.num_people */ const struct race *u_irace(const struct unit *u); const struct race *u_race(const struct unit *u); diff --git a/src/kernel/unit.test.c b/src/kernel/unit.test.c index 3932bb239..caa8a9b5c 100644 --- a/src/kernel/unit.test.c +++ b/src/kernel/unit.test.c @@ -531,7 +531,7 @@ static void test_unlimited_units(CuTest *tc) { CuAssertIntEquals(tc, 0, f->num_units); CuAssertIntEquals(tc, 0, f->num_people); u = test_create_unit(f, test_create_region(0, 0, NULL)); -// CuAssertTrue(tc, count_unit(u)); + CuAssertTrue(tc, count_unit(u)); CuAssertIntEquals(tc, 1, f->num_units); CuAssertIntEquals(tc, 1, f->num_people); u_setfaction(u, NULL); @@ -541,6 +541,7 @@ static void test_unlimited_units(CuTest *tc) { CuAssertIntEquals(tc, 1, f->num_units); CuAssertIntEquals(tc, 1, f->num_people); u_setrace(u, rc); + CuAssertTrue(tc, !count_unit(u)); CuAssertIntEquals(tc, 0, f->num_units); CuAssertIntEquals(tc, 0, f->num_people); scale_number(u, 10); @@ -549,6 +550,9 @@ static void test_unlimited_units(CuTest *tc) { u_setrace(u, f->race); CuAssertIntEquals(tc, 1, f->num_units); CuAssertIntEquals(tc, 10, f->num_people); + remove_unit(&u->region->units, u); + CuAssertIntEquals(tc, 0, f->num_units); + CuAssertIntEquals(tc, 0, f->num_people); test_cleanup(); } From 627f5ba2d159e2b3efa1970bb3ec030e8c93e7a8 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sun, 12 Mar 2017 14:18:42 +0100 Subject: [PATCH 7/8] BUG 2313: summary should not reset num_people. it did not count spells correctly. --- src/summary.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/summary.c b/src/summary.c index 71bb30efc..1f4933696 100644 --- a/src/summary.c +++ b/src/summary.c @@ -394,7 +394,6 @@ summary *make_summary(void) } ++plang->number; f->nregions = 0; - f->num_people = 0; if (f->units) { s->factions++; /* Problem mit Monsterpartei ... */ @@ -476,7 +475,6 @@ summary *make_summary(void) } } - f->num_people += u->number; orace = (int)old_race(u_race(u)); if (orace >= 0) { s->poprace[orace] += u->number; From 7b4550b9d6498f527c14fd39d5404e3cce3a9f25 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sun, 12 Mar 2017 14:20:34 +0100 Subject: [PATCH 8/8] kill faction.nregions, it is calculated but never read. --- src/kernel/faction.h | 1 - src/summary.c | 10 ---------- 2 files changed, 11 deletions(-) diff --git a/src/kernel/faction.h b/src/kernel/faction.h index 9b290e5da..c6afc767c 100644 --- a/src/kernel/faction.h +++ b/src/kernel/faction.h @@ -81,7 +81,6 @@ extern "C" { int options; 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; score_t score; struct alliance *alliance; int alliance_joindate; /* the turn on which the faction joined its current alliance (or left the last one) */ diff --git a/src/summary.c b/src/summary.c index 1f4933696..aafdfe081 100644 --- a/src/summary.c +++ b/src/summary.c @@ -393,7 +393,6 @@ summary *make_summary(void) plang->locale = lang; } ++plang->number; - f->nregions = 0; if (f->units) { s->factions++; /* Problem mit Monsterpartei ... */ @@ -432,11 +431,6 @@ summary *make_summary(void) s->peasants += rpeasants(r); s->peasantmoney += rmoney(r); - /* Einheiten Info. nregions darf nur einmal pro Partei - * incrementiert werden. */ - - for (u = r->units; u; u = u->next) - freset(u->faction, FFL_SELECT); for (u = r->units; u; u = u->next) { int orace; f = u->faction; @@ -469,10 +463,6 @@ summary *make_summary(void) if (aktskill > s->maxskill) s->maxskill = aktskill; } - if (!fval(f, FFL_SELECT)) { - f->nregions++; - fset(f, FFL_SELECT); - } } orace = (int)old_race(u_race(u));