From 2e3c0db02aa8d5441cfdb9dd55f162f37cbcac59 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 5 Sep 2015 18:14:28 +0200 Subject: [PATCH 1/7] add a failing test for bug 1685 --- src/give.test.c | 26 ++++++++++++++++++++++++++ src/kernel/config.c | 10 ++++++++++ src/kernel/config.h | 1 + src/tests.c | 1 + 4 files changed, 38 insertions(+) diff --git a/src/give.test.c b/src/give.test.c index a5714f445..b79214040 100644 --- a/src/give.test.c +++ b/src/give.test.c @@ -296,6 +296,31 @@ static void test_give_denied_by_rules(CuTest * tc) { test_cleanup(); } +static void test_give_invalid_target(CuTest *tc) { + // bug https://bugs.eressea.de/view.php?id=1685 + struct give env; + order *ord; + struct locale * lang; + + test_cleanup(); + env.f1 = test_create_faction(0); + env.f2 = 0; + setup_give(&env); + + i_change(&env.src->items, env.itype, 10); + lang = get_or_create_locale("test"); + env.f1->locale = lang; + locale_setstring(lang, "KRAEUTER", "HERBS"); + init_locale(lang); + ord = create_order(K_GIVE, lang, "## HERBS"); + assert(ord); + + give_cmd(env.src, ord); + CuAssertIntEquals(tc, 10, i_get(env.src->items, env.itype)); + CuAssertPtrNotNull(tc, test_find_messagetype(env.f1->msgs, "feedback_unit_not_found")); + test_cleanup(); +} + CuSuite *get_give_suite(void) { CuSuite *suite = CuSuiteNew(); @@ -315,5 +340,6 @@ CuSuite *get_give_suite(void) SUITE_ADD_TEST(suite, test_give_herbs); SUITE_ADD_TEST(suite, test_give_okay); SUITE_ADD_TEST(suite, test_give_denied_by_rules); + SUITE_ADD_TEST(suite, test_give_invalid_target); return suite; } diff --git a/src/kernel/config.c b/src/kernel/config.c index 573126137..a63fb092d 100644 --- a/src/kernel/config.c +++ b/src/kernel/config.c @@ -1025,6 +1025,16 @@ typedef struct param { char *data; } param; +void free_params(struct param **pp) { + while (*pp) { + param *p = *pp; + free(p->name); + free(p->data); + *pp = p->next; + free(p); + } +} + const char *get_param(const struct param *p, const char *key) { while (p != NULL) { diff --git a/src/kernel/config.h b/src/kernel/config.h index 056de8f84..2f71f7e01 100644 --- a/src/kernel/config.h +++ b/src/kernel/config.h @@ -277,6 +277,7 @@ extern "C" { int get_param_int(const struct param *p, const char *key, int def); int check_param(const struct param *p, const char *key, const char *searchvalue); float get_param_flt(const struct param *p, const char *key, float def); + void free_params(struct param **pp); bool ExpensiveMigrants(void); int NMRTimeout(void); diff --git a/src/tests.c b/src/tests.c index bcbd18c29..fe0e7bfcd 100644 --- a/src/tests.c +++ b/src/tests.c @@ -73,6 +73,7 @@ void test_cleanup(void) free_resources(); global.functions.maintenance = NULL; global.functions.wage = NULL; + free_params(&global.parameters); default_locale = 0; free_locales(); free_spells(); From b631f539ce37ab3d571f11a46c27cd3eecad44f2 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 5 Sep 2015 18:18:22 +0200 Subject: [PATCH 2/7] simplify atoi36, it does not need to eat whitespace --- src/util/base36.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/util/base36.c b/src/util/base36.c index c6ae9f02e..65c9db812 100644 --- a/src/util/base36.c +++ b/src/util/base36.c @@ -32,9 +32,6 @@ int atoi36(const char *str) assert(s); if (!(*s)) return 0; - - while (isxspace(*(unsigned char *)s)) - ++s; if (*s == '-') { sign = -1; ++s; From 74d7caf526ba38d5b76b93cb9a99d557e93b1ebf Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 5 Sep 2015 19:24:59 +0200 Subject: [PATCH 3/7] remove dead code for base10->base36 conversion --- src/report.c | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/report.c b/src/report.c index b63b45755..f62e3d728 100644 --- a/src/report.c +++ b/src/report.c @@ -2448,21 +2448,6 @@ const char *charset) return 0; } -void base36conversion(void) -{ - region *r; - for (r = regions; r; r = r->next) { - unit *u; - for (u = r->units; u; u = u->next) { - if (forbiddenid(u->no)) { - uunhash(u); - u->no = newunitid(); - uhash(u); - } - } - } -} - #define FMAXHASH 1021 struct fsee { From 400b8f1ffad1fd3d28d7586280b0a6ec9329e357 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 5 Sep 2015 19:25:55 +0200 Subject: [PATCH 4/7] add another failing test for bug 1685 add test for forbiddenid (cannot have a unit with id TEMP) --- src/kernel/config.c | 1 + src/kernel/config.test.c | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/src/kernel/config.c b/src/kernel/config.c index a63fb092d..b51cf40ee 100644 --- a/src/kernel/config.c +++ b/src/kernel/config.c @@ -842,6 +842,7 @@ building *largestbuilding(const region * r, cmp_building_cb cmp_gt, extern faction *dfindhash(int i); static const char *forbidden[] = { "t", "te", "tem", "temp", NULL }; +// PEASANT: "b", "ba", "bau", "baue", "p", "pe", "pea", "peas" int forbiddenid(int id) { diff --git a/src/kernel/config.test.c b/src/kernel/config.test.c index ef5792bd0..8ed225907 100644 --- a/src/kernel/config.test.c +++ b/src/kernel/config.test.c @@ -53,6 +53,13 @@ static void test_getunit(CuTest *tc) { CuAssertPtrEquals(tc, NULL, u2); free_order(ord); + // bug https://bugs.eressea.de/view.php?id=1685 + ord = create_order(K_GIVE, lang, "##"); + init_order(ord); + CuAssertIntEquals(tc, GET_NOTFOUND, getunit(u->region, u->faction, &u2)); + CuAssertPtrEquals(tc, NULL, u2); + free_order(ord); + ord = create_order(K_GIVE, lang, "TEMP 42"); init_order(ord); CuAssertIntEquals(tc, GET_UNIT, getunit(u->region, u->faction, &u2)); @@ -97,9 +104,20 @@ static void test_param_flt(CuTest * tc) CuAssertDblEquals(tc, 42.0, get_param_flt(par, "bar", 0.0), 0.01); } +static void test_forbiddenid(CuTest *tc) { + CuAssertIntEquals(tc, 0, forbiddenid(1)); + CuAssertIntEquals(tc, 1, forbiddenid(0)); + CuAssertIntEquals(tc, 1, forbiddenid(-1)); + CuAssertIntEquals(tc, 1, forbiddenid(atoi36("temp"))); + CuAssertIntEquals(tc, 1, forbiddenid(atoi36("tem"))); + CuAssertIntEquals(tc, 1, forbiddenid(atoi36("te"))); + CuAssertIntEquals(tc, 1, forbiddenid(atoi36("t"))); +} + CuSuite *get_config_suite(void) { CuSuite *suite = CuSuiteNew(); + SUITE_ADD_TEST(suite, test_forbiddenid); SUITE_ADD_TEST(suite, test_getunit); SUITE_ADD_TEST(suite, test_get_set_param); SUITE_ADD_TEST(suite, test_param_int); From 3dc173b6ec1403356bd4508dbf14b040187084ae Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 5 Sep 2015 19:44:30 +0200 Subject: [PATCH 5/7] add unit tests for read_unitid --- src/kernel/config.test.c | 47 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/src/kernel/config.test.c b/src/kernel/config.test.c index 8ed225907..6169e1722 100644 --- a/src/kernel/config.test.c +++ b/src/kernel/config.test.c @@ -14,6 +14,45 @@ struct critbit_tree; +static void test_read_unitid(CuTest *tc) { + unit *u; + order *ord; + attrib *a; + struct locale *lang; + struct terrain_type *t_plain; + + test_cleanup(); + lang = get_or_create_locale("de"); + test_translate_param(lang, P_TEMP, "TEMP"); + /* note that the english order is FIGHT, not COMBAT, so this is a poor example */ + t_plain = test_create_terrain("plain", LAND_REGION); + u = test_create_unit(test_create_faction(0), test_create_region(0, 0, t_plain)); + a = a_add(&u->attribs, a_new(&at_alias)); + a->data.i = atoi36("42"); /* this unit is also TEMP 42 */ + + ord = create_order(K_GIVE, lang, "TEMP 42"); + init_order(ord); + CuAssertIntEquals(tc, u->no, read_unitid(u->faction, u->region)); + free_order(ord); + + ord = create_order(K_GIVE, lang, "8"); + init_order(ord); + CuAssertIntEquals(tc, 8, read_unitid(u->faction, u->region)); + free_order(ord); + + ord = create_order(K_GIVE, lang, ""); + init_order(ord); + CuAssertIntEquals(tc, -1, read_unitid(u->faction, u->region)); + free_order(ord); + + ord = create_order(K_GIVE, lang, "TEMP"); + init_order(ord); + CuAssertIntEquals(tc, -1, read_unitid(u->faction, u->region)); + free_order(ord); + + test_cleanup(); +} + static void test_getunit(CuTest *tc) { unit *u, *u2; order *ord; @@ -53,6 +92,13 @@ static void test_getunit(CuTest *tc) { CuAssertPtrEquals(tc, NULL, u2); free_order(ord); + // bug https://bugs.eressea.de/view.php?id=1685 + ord = create_order(K_GIVE, lang, "TEMP ##"); + init_order(ord); + CuAssertIntEquals(tc, GET_NOTFOUND, getunit(u->region, u->faction, &u2)); + CuAssertPtrEquals(tc, NULL, u2); + free_order(ord); + // bug https://bugs.eressea.de/view.php?id=1685 ord = create_order(K_GIVE, lang, "##"); init_order(ord); @@ -119,6 +165,7 @@ CuSuite *get_config_suite(void) CuSuite *suite = CuSuiteNew(); SUITE_ADD_TEST(suite, test_forbiddenid); SUITE_ADD_TEST(suite, test_getunit); + SUITE_ADD_TEST(suite, test_read_unitid); SUITE_ADD_TEST(suite, test_get_set_param); SUITE_ADD_TEST(suite, test_param_int); SUITE_ADD_TEST(suite, test_param_flt); From a2c269e8056c784c562fa2ff85497b04be2fc225 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 5 Sep 2015 19:45:34 +0200 Subject: [PATCH 6/7] add another failing unit test for bug 1685, closer to the core of the problem --- src/kernel/config.test.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/kernel/config.test.c b/src/kernel/config.test.c index 6169e1722..a8894a7f6 100644 --- a/src/kernel/config.test.c +++ b/src/kernel/config.test.c @@ -50,6 +50,12 @@ static void test_read_unitid(CuTest *tc) { CuAssertIntEquals(tc, -1, read_unitid(u->faction, u->region)); free_order(ord); + // bug https://bugs.eressea.de/view.php?id=1685 + ord = create_order(K_GIVE, lang, "##"); + init_order(ord); + CuAssertIntEquals(tc, -1, read_unitid(u->faction, u->region)); + free_order(ord); + test_cleanup(); } From 79663d5933c15b59b3c74efeb323f3228eacd465 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 5 Sep 2015 19:48:25 +0200 Subject: [PATCH 7/7] relatively simple fix to bug 1685: unit ids must start with alphanumerical characters. https://bugs.eressea.de/view.php?id=1685 --- src/kernel/config.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kernel/config.c b/src/kernel/config.c index b51cf40ee..fe4d96934 100644 --- a/src/kernel/config.c +++ b/src/kernel/config.c @@ -768,7 +768,7 @@ int read_unitid(const faction * f, const region * r) * paramliste. machen wir das nicht, dann wird getnewunit in s nach der * nummer suchen, doch dort steht bei temp-units nur "temp" drinnen! */ - if (!s || *s == 0) { + if (!s || *s == 0 || !isalnum(*s)) { return -1; } if (isparam(s, f->locale, P_TEMP)) {