From c9445ab517acde15a659c0441796c473a077384a Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Mon, 18 Sep 2017 11:48:42 +0200 Subject: [PATCH 1/3] change resource_type trie storage --- src/kernel/item.c | 56 +++++++++++++++++++++++++++++++---------------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/src/kernel/item.c b/src/kernel/item.c index 8639ac5d1..7f77fc3bb 100644 --- a/src/kernel/item.c +++ b/src/kernel/item.c @@ -60,6 +60,12 @@ static critbit_tree cb_resources; luxury_type *luxurytypes; potion_type *potiontypes; +#define RTYPENAMELEN 16 +typedef struct rt_entry { + char key[RTYPENAMELEN]; + struct resource_type *value; +} rt_entry; + static int res_changeaura(unit * u, const resource_type * rtype, int delta) { assert(rtype != NULL); @@ -166,13 +172,19 @@ static int num_resources; static void rt_register(resource_type * rtype) { - char buffer[64]; const char * name = rtype->_name; size_t len = strlen(name); + rt_entry ent; - assert(len < sizeof(buffer) - sizeof(rtype)); - len = cb_new_kv(name, len, &rtype, sizeof(rtype), buffer); - cb_insert(&cb_resources, buffer, len); + if (len > RTYPENAMELEN) { + log_error("resource names may not be longer than %d bytes: %s", + RTYPENAMELEN, name); + len = RTYPENAMELEN; + } + ent.value = rtype; + memset(ent.key, 0, RTYPENAMELEN); + memcpy(ent.key, name, len); + cb_insert(&cb_resources, &ent, sizeof(ent)); ++num_resources; } @@ -186,7 +198,6 @@ resource_type *rt_get_or_create(const char *name) { else { rtype->_name = strdup(name); rt_register(rtype); - return rt_find(name); } } return rtype; @@ -390,12 +401,18 @@ const potion_type *resource2potion(const resource_type * rtype) resource_type *rt_find(const char *name) { void * match; - resource_type *result = 0; + size_t len = strlen(name); - if (cb_find_prefix(&cb_resources, name, strlen(name) + 1, &match, 1, 0)) { - cb_get_kv(match, &result, sizeof(result)); + if (len > RTYPENAMELEN) { + log_error("resource name is longer than $d bytes: %s", + RTYPENAMELEN, name); + len = RTYPENAMELEN; } - return result; + if (cb_find_prefix(&cb_resources, name, len, &match, 1, 0)) { + rt_entry *ent = (rt_entry *)match; + return ent->value; + } + return NULL; } item **i_find(item ** i, const item_type * it) @@ -792,14 +809,14 @@ int change_money(unit * u, int v) return 0; } -static int add_resourcename_cb(const void * match, const void * key, size_t keylen, void *data) +static int add_resourcename_cb(const void * match, const void * key, + size_t keylen, void *data) { struct locale * lang = (struct locale *)data; int i = locale_index(lang); critbit_tree * cb = rnames + i; - resource_type *rtype; + resource_type *rtype = ((rt_entry *)match)->value; - cb_get_kv(match, &rtype, sizeof(rtype)); for (i = 0; i != 2; ++i) { char buffer[128]; const char * name = LOC(lang, resourcename(rtype, (i == 0) ? 0 : NMF_PLURAL)); @@ -835,20 +852,20 @@ const resource_type *findresourcetype(const char *name, const struct locale *lan else { log_debug("findresourcetype: transliterate failed for '%s'\n", name); } - return 0; + return NULL; } attrib_type at_showitem = { "showitem" }; -static int add_itemname_cb(const void * match, const void * key, size_t keylen, void *data) +static int add_itemname_cb(const void * match, const void * key, + size_t keylen, void *data) { struct locale * lang = (struct locale *)data; critbit_tree * cb = inames + locale_index(lang); - resource_type *rtype; + resource_type *rtype = ((rt_entry *)match)->value; - cb_get_kv(match, &rtype, sizeof(rtype)); if (rtype->itype) { int i; for (i = 0; i != 2; ++i) { @@ -980,9 +997,10 @@ void free_rtype(resource_type *rtype) { free(rtype); } -int free_rtype_cb(const void * match, const void * key, size_t keylen, void *cbdata) { - resource_type *rtype; - cb_get_kv(match, &rtype, sizeof(rtype)); +static int free_rtype_cb(const void * match, const void * key, + size_t keylen, void *cbdata) +{ + resource_type *rtype = ((rt_entry *)match)->value;; free_rtype(rtype); return 0; } From 4cab65d23367a913843a27cac470df3e7b3d15a0 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Mon, 18 Sep 2017 17:32:39 +0200 Subject: [PATCH 2/3] fail get_equipment test when name is exactly 16 bytes long. --- src/kernel/equipment.c | 1 + src/kernel/equipment.test.c | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/kernel/equipment.c b/src/kernel/equipment.c index 98d6b1e70..0db84c761 100644 --- a/src/kernel/equipment.c +++ b/src/kernel/equipment.c @@ -204,6 +204,7 @@ void free_ls(void *arg) { static critbit_tree cb_equipments = { 0 }; #define EQNAMELEN 16 + typedef struct eq_entry { char key[EQNAMELEN]; equipment *value; diff --git a/src/kernel/equipment.test.c b/src/kernel/equipment.test.c index 4ed077fb0..cc88fc826 100644 --- a/src/kernel/equipment.test.c +++ b/src/kernel/equipment.test.c @@ -51,12 +51,12 @@ static void test_get_equipment(CuTest * tc) equipment * eq; test_setup(); - eq = create_equipment("catapultammo"); + eq = create_equipment("catapultammo1234"); CuAssertPtrNotNull(tc, eq); - CuAssertStrEquals(tc, "catapultammo", eq->name); - eq = get_equipment("catapultammo"); + CuAssertStrEquals(tc, "catapultammo1234", eq->name); + eq = get_equipment("catapultammo1234"); CuAssertPtrNotNull(tc, eq); - CuAssertStrEquals(tc, "catapultammo", eq->name); + CuAssertStrEquals(tc, "catapultammo1234", eq->name); eq = get_equipment("catapult"); CuAssertPtrEquals(tc, NULL, eq); eq = create_equipment("catapult"); From 2451a8f637a1c3aa94f8881f70e905dfe9a95bdf Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Mon, 18 Sep 2017 17:56:27 +0200 Subject: [PATCH 3/3] resource names are max 23 bytes long. fix rt_find nul-termination bug --- clibs | 2 +- src/kernel/equipment.c | 2 +- src/kernel/item.c | 23 ++++++++++++----------- src/kernel/item.test.c | 8 ++++---- 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/clibs b/clibs index d286006a2..2a55c27fe 160000 --- a/clibs +++ b/clibs @@ -1 +1 @@ -Subproject commit d286006a28c8aa7cd70ed7fd4cd172b50ade9727 +Subproject commit 2a55c27fedec76845cf82c758b7b7c3fa649c286 diff --git a/src/kernel/equipment.c b/src/kernel/equipment.c index 6bc7e1616..73d4e7d3a 100644 --- a/src/kernel/equipment.c +++ b/src/kernel/equipment.c @@ -212,7 +212,7 @@ typedef struct eq_entry { equipment *get_equipment(const char *eqname) { - const char *match; + char *match; assert(strlen(eqname) < EQNAMELEN); diff --git a/src/kernel/item.c b/src/kernel/item.c index 2c38db008..ec162fa7b 100644 --- a/src/kernel/item.c +++ b/src/kernel/item.c @@ -60,7 +60,7 @@ static critbit_tree cb_resources; luxury_type *luxurytypes; potion_type *potiontypes; -#define RTYPENAMELEN 16 +#define RTYPENAMELEN 24 typedef struct rt_entry { char key[RTYPENAMELEN]; struct resource_type *value; @@ -176,10 +176,10 @@ static void rt_register(resource_type * rtype) size_t len = strlen(name); rt_entry ent; - if (len > RTYPENAMELEN) { - log_error("resource names may not be longer than %d bytes: %s", - RTYPENAMELEN, name); - len = RTYPENAMELEN; + if (len >= RTYPENAMELEN) { + log_error("resource name is longer than %d bytes: %s", + RTYPENAMELEN-1, name); + len = RTYPENAMELEN-1; } ent.value = rtype; memset(ent.key, 0, RTYPENAMELEN); @@ -389,15 +389,16 @@ const potion_type *resource2potion(const resource_type * rtype) resource_type *rt_find(const char *name) { - void * match; + char *match; size_t len = strlen(name); - if (len > RTYPENAMELEN) { - log_error("resource name is longer than $d bytes: %s", - RTYPENAMELEN, name); - len = RTYPENAMELEN; + if (len >= RTYPENAMELEN) { + log_error("resource name is longer than %d bytes: %s", + RTYPENAMELEN-1, name); + return NULL; } - if (cb_find_prefix(&cb_resources, name, len, &match, 1, 0)) { + match = cb_find_str(&cb_resources, name); + if (match) { rt_entry *ent = (rt_entry *)match; return ent->value; } diff --git a/src/kernel/item.test.c b/src/kernel/item.test.c index c6113ec5d..89954c451 100644 --- a/src/kernel/item.test.c +++ b/src/kernel/item.test.c @@ -178,11 +178,11 @@ static void test_get_resource(CuTest *tc) { test_setup(); - CuAssertPtrEquals(tc, NULL, rt_find("catapultammo")); - rtype = rt_get_or_create("catapultammo"); + CuAssertPtrEquals(tc, NULL, rt_find("catapultammo123")); + rtype = rt_get_or_create("catapultammo123"); CuAssertPtrNotNull(tc, rtype); - CuAssertPtrEquals(tc, rtype, rt_find("catapultammo")); - CuAssertStrEquals(tc, "catapultammo", rtype->_name); + CuAssertPtrEquals(tc, rtype, rt_find("catapultammo123")); + CuAssertStrEquals(tc, "catapultammo123", rtype->_name); CuAssertPtrEquals(tc, NULL, rt_find("catapult")); rtype = rt_get_or_create("catapult");