From 2e7a1a3ded4ea2a54b9e258b500569781874d05c Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Mon, 9 Nov 2015 18:03:19 +0100 Subject: [PATCH 1/9] CID 32208 Time of check time of use --- src/reports.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/reports.c b/src/reports.c index 27f3a5c28..ca1cfb0f3 100644 --- a/src/reports.c +++ b/src/reports.c @@ -1541,8 +1541,8 @@ static void prepare_report(struct report_context *ctx, faction *f) } static void mkreportdir(const char *rpath) { - if (_access(rpath, 0) < 0) { - if (_mkdir(rpath) != 0) { + if (_mkdir(rpath) != 0) { + if (_access(rpath, 0) < 0) { log_error("could not create reports directory %s: %s", rpath, strerror(errno)); abort(); } From 9ff6aa0d4227c7fc5e819d27c69c3dc476b3be0c Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Mon, 9 Nov 2015 18:06:11 +0100 Subject: [PATCH 2/9] CID 22472 Division or modulo by zero CID 22471 Division or modulo by zero github issue #350 github issue #349 --- src/market.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/market.c b/src/market.c index a4158e48d..68ef9767c 100644 --- a/src/market.c +++ b/src/market.c @@ -96,8 +96,8 @@ void do_markets(void) const struct race *rc = f ? f->race : NULL; int p = rpeasants(r); int numlux = rc_luxury_trade(rc), numherbs = rc_herb_trade(rc); - numlux = (p + numlux - MIN_PEASANTS) / numlux; - numherbs = (p + numherbs - MIN_PEASANTS) / numherbs; + if (numlux>0) numlux = (p + numlux - MIN_PEASANTS) / numlux; + if (numherbs>0) numherbs = (p + numherbs - MIN_PEASANTS) / numherbs; if (numlux > 0 || numherbs > 0) { int d, nmarkets = 0; const item_type *lux = r_luxury(r); From fed660987a26128b483b2981e2a91faa0281e7eb Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Mon, 9 Nov 2015 19:11:47 +0100 Subject: [PATCH 3/9] CID 22519 et al (resource leaks) make the failure case for get_spellbook a little slower, but make static analysis happy. --- src/magic.c | 2 ++ src/magic.test.c | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/magic.c b/src/magic.c index d65685aff..20fd47ccd 100644 --- a/src/magic.c +++ b/src/magic.c @@ -2985,6 +2985,8 @@ spellbook * get_spellbook(const char * name) log_error("cb_insert failed although cb_find returned nothing for spellbook=%s", name); assert(!"should not happen"); } + cb_find_prefix(&cb_spellbooks, name, strlen(name), &match, 1, 0); + cb_get_kv(match, &result, sizeof(result)); } return result; } diff --git a/src/magic.test.c b/src/magic.test.c index f356fa686..b208836f9 100644 --- a/src/magic.test.c +++ b/src/magic.test.c @@ -53,8 +53,9 @@ void test_spellbooks(CuTest * tc) test_cleanup(); herp = get_spellbook("herp"); - derp = get_spellbook("derp"); CuAssertPtrNotNull(tc, herp); + CuAssertPtrEquals(tc, herp, get_spellbook("herp")); + derp = get_spellbook("derp"); CuAssertPtrNotNull(tc, derp); CuAssertTrue(tc, derp != herp); CuAssertStrEquals(tc, "herp", herp->name); From ea8721367a8fb8e213900752e28e4187b9f7639c Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Mon, 9 Nov 2015 19:43:51 +0100 Subject: [PATCH 4/9] test and fix shock trigger. https://bugs.eressea.de/view.php?id=2154 --- src/CMakeLists.txt | 1 + src/kernel/unit.test.c | 4 ++-- src/magic.c | 5 +++++ src/test_eressea.c | 1 + src/tests.c | 4 +++- src/triggers/shock.c | 13 +++++++++---- 6 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 037898ed9..ada49aad9 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -210,6 +210,7 @@ set(TESTS_SRC upkeep.test.c spells/flyingship.test.c spells/magicresistance.test.c + triggers/shock.test.c ${ATTRIBUTES_TESTS} ${UTIL_TESTS} ${KERNEL_TESTS} diff --git a/src/kernel/unit.test.c b/src/kernel/unit.test.c index 0fd58a55a..db2751026 100644 --- a/src/kernel/unit.test.c +++ b/src/kernel/unit.test.c @@ -123,11 +123,11 @@ static void test_scale_number(CuTest *tc) { u = test_create_unit(test_create_faction(test_create_race("human")), findregion(0, 0)); change_effect(u, ptype, 1); CuAssertIntEquals(tc, 1, u->number); - CuAssertIntEquals(tc, 1, u->hp); + CuAssertIntEquals(tc, 20, u->hp); CuAssertIntEquals(tc, 1, get_effect(u, ptype)); scale_number(u, 2); CuAssertIntEquals(tc, 2, u->number); - CuAssertIntEquals(tc, 2, u->hp); + CuAssertIntEquals(tc, 40, u->hp); CuAssertIntEquals(tc, 2, get_effect(u, ptype)); set_level(u, SK_ALCHEMY, 1); scale_number(u, 0); diff --git a/src/magic.c b/src/magic.c index 20fd47ccd..ce69df78f 100644 --- a/src/magic.c +++ b/src/magic.c @@ -995,6 +995,11 @@ cancast(unit * u, const spell * sp, int level, int range, struct order * ord) if (reslist != NULL) { ADDMSG(&u->faction->msgs, msg_feedback(u, ord, "missing_components_list", "list", reslist)); + while (reslist) { + resource *res = reslist->next; + free(reslist); + reslist = res; + } return false; } return true; diff --git a/src/test_eressea.c b/src/test_eressea.c index c04764992..8f5c1faa0 100644 --- a/src/test_eressea.c +++ b/src/test_eressea.c @@ -127,6 +127,7 @@ int RunAllTests(int argc, char *argv[]) ADD_SUITE(wormhole); ADD_SUITE(spy); ADD_SUITE(study); + ADD_SUITE(shock); if (suites) { CuSuite *summary = CuSuiteNew(); diff --git a/src/tests.c b/src/tests.c index 8cf97bede..e7518211e 100644 --- a/src/tests.c +++ b/src/tests.c @@ -35,6 +35,8 @@ struct race *test_create_race(const char *name) { race *rc = rc_get_or_create(name); rc->maintenance = 10; + rc->hitpoints = 20; + rc->maxaura = 1.0; rc->ec_flags |= GETITEM; return rc; } @@ -59,7 +61,7 @@ struct region *test_create_region(int x, int y, const terrain_type *terrain) struct faction *test_create_faction(const struct race *rc) { - faction *f = addfaction("nobody@eressea.de", NULL, rc ? rc : rc_get_or_create("human"), default_locale, 0); + faction *f = addfaction("nobody@eressea.de", NULL, rc ? rc : test_create_race("human"), default_locale, 0); return f; } diff --git a/src/triggers/shock.c b/src/triggers/shock.c index fd52c49b5..95e7b61b6 100644 --- a/src/triggers/shock.c +++ b/src/triggers/shock.c @@ -59,13 +59,18 @@ static void do_shock(unit * u, const char *reason) if (u->number > 0) { /* HP - Verlust */ - u->hp = (unit_max_hp(u) * u->number) / 10; - u->hp = _max(1, u->hp); + int hp = (unit_max_hp(u) * u->number) / 10; + hp = _min(u->hp, hp); + u->hp = _max(1, hp); } /* Aura - Verlust */ if (is_mage(u)) { - set_spellpoints(u, max_spellpoints(u->region, u) / 10); + int aura = max_spellpoints(u->region, u) / 10; + int now = get_spellpoints(u); + if (now > aura) { + set_spellpoints(u, aura); + } } /* Evt. Talenttageverlust */ @@ -86,7 +91,7 @@ static void do_shock(unit * u, const char *reason) } if (u->faction != NULL) { ADDMSG(&u->faction->msgs, msg_message("shock", - "mage reason", u, _strdup(reason))); + "mage reason", u, reason)); } } From 32c777d4286d3dbbd73b197ccd6ed32dc981ddc1 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Mon, 9 Nov 2015 19:46:01 +0100 Subject: [PATCH 5/9] add missing test file, try again --- src/triggers/shock.test.c | 53 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 src/triggers/shock.test.c diff --git a/src/triggers/shock.test.c b/src/triggers/shock.test.c new file mode 100644 index 000000000..8957c33fc --- /dev/null +++ b/src/triggers/shock.test.c @@ -0,0 +1,53 @@ +#include +#include "shock.h" +#include "../magic.h" + +#include +#include +#include + +#include +#include + +static void test_shock(CuTest *tc) { + unit *u; + trigger *tt; + test_cleanup(); + + u = test_create_unit(test_create_faction(0), test_create_region(0, 0, 0)); + create_mage(u, M_GRAY); + set_level(u, SK_MAGIC, 5); + set_spellpoints(u, 10); + u->hp = 10; + tt = trigger_shock(u); + tt->type->handle(tt, u); + CuAssertIntEquals(tc, 2, u->hp); + CuAssertIntEquals(tc, 2, get_spellpoints(u)); + CuAssertPtrNotNull(tc, test_find_messagetype(u->faction->msgs, "shock")); + test_cleanup(); +} + +static void test_shock_low(CuTest *tc) { + unit *u; + trigger *tt; + test_cleanup(); + + u = test_create_unit(test_create_faction(0), test_create_region(0, 0, 0)); + create_mage(u, M_GRAY); + set_level(u, SK_MAGIC, 5); + set_spellpoints(u, 1); + u->hp = 1; + tt = trigger_shock(u); + tt->type->handle(tt, u); + CuAssertIntEquals(tc, 1, u->hp); + CuAssertIntEquals(tc, 1, get_spellpoints(u)); + test_cleanup(); +} + +CuSuite *get_shock_suite(void) +{ + CuSuite *suite = CuSuiteNew(); + SUITE_ADD_TEST(suite, test_shock); + SUITE_ADD_TEST(suite, test_shock_low); + return suite; +} From 66e43caf9d83193575387429459e8ca09bf3db77 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Mon, 9 Nov 2015 20:03:38 +0100 Subject: [PATCH 6/9] CID 32303 Unchecked return value appeasing coverity --- src/magic.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/magic.c b/src/magic.c index ce69df78f..1873bb041 100644 --- a/src/magic.c +++ b/src/magic.c @@ -2978,7 +2978,7 @@ spellbook * get_spellbook(const char * name) spellbook * result; void * match; - if (cb_find_prefix(&cb_spellbooks, name, strlen(name), &match, 1, 0)) { + if (cb_find_prefix(&cb_spellbooks, name, strlen(name), &match, 1, 0) > 0) { cb_get_kv(match, &result, sizeof(result)); } else { @@ -2990,8 +2990,10 @@ spellbook * get_spellbook(const char * name) log_error("cb_insert failed although cb_find returned nothing for spellbook=%s", name); assert(!"should not happen"); } - cb_find_prefix(&cb_spellbooks, name, strlen(name), &match, 1, 0); - cb_get_kv(match, &result, sizeof(result)); + result = 0; + if (cb_find_prefix(&cb_spellbooks, name, strlen(name), &match, 1, 0) > 0) { + cb_get_kv(match, &result, sizeof(result)); + } } return result; } From e14cc5025264be2b7df191de255d521dfeca05c0 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Mon, 9 Nov 2015 20:05:55 +0100 Subject: [PATCH 7/9] CID 22522, 22520 Resource leak helping coverity scan to understand this code --- src/kernel/item.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/kernel/item.c b/src/kernel/item.c index ca6c2f32d..1b312c655 100644 --- a/src/kernel/item.c +++ b/src/kernel/item.c @@ -187,6 +187,7 @@ resource_type *rt_get_or_create(const char *name) { else { rtype->_name = _strdup(name); rt_register(rtype); + return rt_find(name); } } return rtype; From edb862a8fcbd79089cc0044395b7f3fa4494b6d2 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Mon, 9 Nov 2015 20:08:58 +0100 Subject: [PATCH 8/9] CID 22461 Division or modulo by zero partial fix github issue #326 also fewer calls to rmoney --- src/economy.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/economy.c b/src/economy.c index c6e0d360a..9a9c5857a 100644 --- a/src/economy.c +++ b/src/economy.c @@ -2898,11 +2898,12 @@ static void expandloot(region * r, request * lootorders) if (!norders) return; - for (i = 0; i != norders && rmoney(r) > TAXFRACTION * 2; i++) { + for (i = 0; i != norders && startmoney > TAXFRACTION * 2; i++) { change_money(oa[i].unit, TAXFRACTION); oa[i].unit->n += TAXFRACTION; /*Looting destroys double the money*/ - rsetmoney(r, rmoney(r) - TAXFRACTION * 2); + startmoney = startmoney - TAXFRACTION * 2; + rsetmoney(r, startmoney); looted = looted + TAXFRACTION * 2; } free(oa); From 34c9958368f22b4475313ef52eb08b27cf6c09c4 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Tue, 10 Nov 2015 10:44:17 +0100 Subject: [PATCH 9/9] CID 22584 Result is not floating-point github issue #326 chance of morale loss is 20-100 percent depending on loss of silver --- src/economy.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/economy.c b/src/economy.c index 9a9c5857a..19223daa1 100644 --- a/src/economy.c +++ b/src/economy.c @@ -2898,18 +2898,17 @@ static void expandloot(region * r, request * lootorders) if (!norders) return; - for (i = 0; i != norders && startmoney > TAXFRACTION * 2; i++) { + for (i = 0; i != norders && startmoney > looted + TAXFRACTION * 2; i++) { change_money(oa[i].unit, TAXFRACTION); oa[i].unit->n += TAXFRACTION; /*Looting destroys double the money*/ - startmoney = startmoney - TAXFRACTION * 2; - rsetmoney(r, startmoney); - looted = looted + TAXFRACTION * 2; + looted += TAXFRACTION * 2; } + rsetmoney(r, startmoney - looted); free(oa); /* Lowering morale by 1 depending on the looted money (+20%) */ - if (rng_int() % 100 < ((looted / startmoney) + 0.2)) { + if (rng_int() % 100 < 20 + (looted * 80) / startmoney) { int m = region_get_morale(r); if (m) { /*Nur Moral -1, turns is not changed, so the first time nothing happens if the morale is good*/