diff --git a/src/bindings/bind_region.c b/src/bindings/bind_region.c index c98849dfa..8404b2d92 100644 --- a/src/bindings/bind_region.c +++ b/src/bindings/bind_region.c @@ -319,8 +319,9 @@ static critbit_tree * special_resources(void) char buffer[32]; int i; for (i=0;special[i];++i) { - cb_new_kv(special[i], &i, sizeof(int), buffer); - cb_insert(&cb, buffer, strlen(special[i])+1+sizeof(int)); + size_t len = strlen(special[i]); + len = cb_new_kv(special[i], len, &i, sizeof(int), buffer); + cb_insert(&cb, buffer, len); } } return &cb; diff --git a/src/kernel/config.c b/src/kernel/config.c index 6d1d753fd..7033e4ba8 100644 --- a/src/kernel/config.c +++ b/src/kernel/config.c @@ -1989,11 +1989,12 @@ static void init_translations(const struct locale *lang, int ut, const char * (* char * str = transliterate(buffer, sizeof(buffer)-sizeof(int), key); if (str) { critbit_tree * cb = (critbit_tree *)*tokens; - if (!cb) { + size_t len = strlen(str); + if (!cb) { *tokens = cb = (critbit_tree *)calloc(1, sizeof(critbit_tree *)); } - cb_new_kv(str, &i, sizeof(int), buffer); - cb_insert(cb, buffer, strlen(str)+1+sizeof(int)); + len = cb_new_kv(str, len, &i, sizeof(int), buffer); + cb_insert(cb, buffer, len); } else { log_error("could not transliterate '%s'\n", key); } diff --git a/src/kernel/item.c b/src/kernel/item.c index 5e842b26b..a432b6226 100644 --- a/src/kernel/item.c +++ b/src/kernel/item.c @@ -177,8 +177,8 @@ void it_register(item_type * itype) size_t len = strlen(name); assert(lenrtype); } } @@ -298,8 +298,8 @@ void rt_register(resource_type * rtype) size_t len = strlen(name); assert(len -#include - /* util includes */ +#include #include #include #include #include #include -quicklist *spells = NULL; +/* libc includes */ +#include +#include -spell * create_spell(const char * name) -{ - spell * sp = (spell *) calloc(1, sizeof(spell)); - sp->sname = strdup(name); - return sp; +static critbit_tree cb_spells; +quicklist * spells; + +void free_spells(void) { + cb_clear(&cb_spells); + ql_free(spells); + spells = 0; } -void register_spell(spell * sp) +spell * create_spell(const char * name, unsigned int id) { - if (sp->id == 0) { - sp->id = hashstring(sp->sname); + spell * sp; + char buffer[64]; + size_t len = strlen(name); + + assert(len+sizeof(sp)id = id ? id : hashstring(name); + sp->sname = strdup(name); + add_spell(&spells, sp); + return sp; + } + free(sp); + return 0; } spell *find_spell(const char *name) { - quicklist *ql = spells; - int qi; + const char * match; + spell * sp = 0; - for (qi = 0; ql; ql_advance(&ql, &qi, 1)) { - spell *sp = (spell *) ql_get(ql, qi); - if (strcmp(name, sp->sname) == 0) { - return sp; - } + match = cb_find_str(&cb_spells, name); + if (match) { + cb_get_kv(match, &sp, sizeof(sp)); + } else { + log_warning("find_spell: could not find spell '%s'\n", name); } - log_warning("find_spell: could not find spell '%s'\n", name); - return 0; + return sp; } /* ------------------------------------------------------------- */ diff --git a/src/kernel/spell.h b/src/kernel/spell.h index 4e5daf1b5..226de96e4 100644 --- a/src/kernel/spell.h +++ b/src/kernel/spell.h @@ -22,16 +22,6 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. extern "C" { #endif - struct fighter; - struct spell; - struct border_type; - struct attrib_type; - struct curse_type; - struct castorder; - struct curse; - struct region; - struct unit; - /* Prototypen */ int use_item_power(struct region *r, struct unit *u); @@ -44,15 +34,14 @@ extern "C" { extern struct attrib_type at_unitdissolve; extern struct attrib_type at_wdwpyramid; - extern struct quicklist *spells; - - extern struct spell * create_spell(const char * name); - extern void register_spell(struct spell *sp); - extern struct spell *find_spell(const char *name); - extern struct spell *find_spellbyid(unsigned int i); - extern struct spell *get_spellfromtoken(struct sc_mage *mage, const char *s, + extern struct spell * create_spell(const char * name, unsigned int id); + extern struct spell * find_spell(const char *name); + extern struct spell * find_spellbyid(unsigned int i); + extern struct spell * get_spellfromtoken(struct sc_mage *mage, const char *s, const struct locale *lang); + extern void free_spells(void); + extern struct quicklist * spells; #ifdef __cplusplus } #endif diff --git a/src/kernel/spell_test.c b/src/kernel/spell_test.c index ebcb74dfe..dc8fc8d47 100644 --- a/src/kernel/spell_test.c +++ b/src/kernel/spell_test.c @@ -1,4 +1,7 @@ +#include + #include +#include #include #include @@ -7,23 +10,48 @@ #include -static void test_register_spell(CuTest * tc) +static void test_create_spell(CuTest * tc) { spell * sp; + test_cleanup(); + CuAssertPtrEquals(tc, 0, spells); CuAssertPtrEquals(tc, 0, find_spell("testspell")); - CuAssertPtrEquals(tc, spells, 0); - sp = create_spell("testspell"); - sp->magietyp = 5; - register_spell(sp); - CuAssertIntEquals(tc, 1, ql_length(spells)); + sp = create_spell("testspell", 0); CuAssertPtrEquals(tc, sp, find_spell("testspell")); + CuAssertPtrNotNull(tc, spells); +} + +static void test_create_duplicate_spell(CuTest * tc) +{ + spell *sp; + + test_cleanup(); + CuAssertPtrEquals(tc, 0, find_spell("testspell")); + + sp = create_spell("testspell", 0); + CuAssertPtrEquals(tc, 0, create_spell("testspell", 0)); + CuAssertPtrEquals(tc, sp, find_spell("testspell")); +} + +static void test_create_spell_with_id(CuTest * tc) +{ + spell *sp; + + test_cleanup(); + CuAssertPtrEquals(tc, 0, find_spellbyid(42)); + sp = create_spell("testspell", 42); + CuAssertPtrEquals(tc, sp, find_spellbyid(42)); + CuAssertPtrEquals(tc, 0, create_spell("testspell", 47)); + CuAssertPtrEquals(tc, 0, find_spellbyid(47)); } CuSuite *get_spell_suite(void) { CuSuite *suite = CuSuiteNew(); - SUITE_ADD_TEST(suite, test_register_spell); + SUITE_ADD_TEST(suite, test_create_spell); + SUITE_ADD_TEST(suite, test_create_duplicate_spell); + SUITE_ADD_TEST(suite, test_create_spell_with_id); return suite; } diff --git a/src/kernel/xmlreader.c b/src/kernel/xmlreader.c index e57176d6c..123c1cf04 100644 --- a/src/kernel/xmlreader.c +++ b/src/kernel/xmlreader.c @@ -1481,13 +1481,14 @@ static int parse_spells(xmlDocPtr doc) int k; spell_component *component; spell *sp; - int valid = 1; + unsigned int index; static int modes[] = { 0, PRECOMBATSPELL, COMBATSPELL, POSTCOMBATSPELL }; /* spellname */ + index = xml_ivalue(node, "index", 0); propValue = xmlGetProp(node, BAD_CAST "name"); assert(propValue != NULL); - sp = create_spell((const char *)propValue); + sp = create_spell((const char *)propValue, index); xmlFree(propValue); propValue = xmlGetProp(node, BAD_CAST "parameters"); @@ -1513,7 +1514,6 @@ static int parse_spells(xmlDocPtr doc) xmlFree(propValue); /* level, rank and flags */ - sp->id = xml_ivalue(node, "index", 0); sp->level = xml_ivalue(node, "level", -1); sp->rank = (char)xml_ivalue(node, "rank", -1); if (xml_bvalue(node, "los", false)) @@ -1544,7 +1544,6 @@ static int parse_spells(xmlDocPtr doc) cast = get_function(sp->sname); if (!cast) { log_error("no spell cast function registered for '%s'\n", sp->sname); - valid = 0; } strlcpy(zText+7, sp->sname, sizeof(zText)-7); fumble = get_function(zText); @@ -1558,10 +1557,8 @@ static int parse_spells(xmlDocPtr doc) if (strcmp((const char *)propValue, "cast") == 0) { if (fun) { cast = fun; - valid = 1; } else { log_error("unknown function name '%s' for spell '%s'\n", (const char *)propValue, sp->sname); - valid = 0; } } else if (fun && strcmp((const char *)propValue, "fumble") == 0) { fumble = fun; @@ -1576,49 +1573,43 @@ static int parse_spells(xmlDocPtr doc) xmlXPathFreeObject(result); } - if (valid) { - /* reading eressea/spells/spell/resource */ - xpath->node = node; - result = xmlXPathEvalExpression(BAD_CAST "resource", xpath); - if (result->nodesetval->nodeNr) { - sp->components = - (spell_component *) malloc(sizeof(spell_component) * - (result->nodesetval->nodeNr + 1)); - sp->components[result->nodesetval->nodeNr].type = 0; - } - for (component = sp->components, k = 0; k != result->nodesetval->nodeNr; - ++k) { - const resource_type *rtype; - xmlNodePtr node = result->nodesetval->nodeTab[k]; - propValue = xmlGetProp(node, BAD_CAST "name"); - assert(propValue); - rtype = rt_find((const char *)propValue); - if (!rtype) { - log_error("spell %s uses unknown component %s.\n", sp->sname, (const char *)propValue); - xmlFree(propValue); - continue; - } - component->type = rtype; + /* reading eressea/spells/spell/resource */ + xpath->node = node; + result = xmlXPathEvalExpression(BAD_CAST "resource", xpath); + if (result->nodesetval->nodeNr) { + sp->components = + (spell_component *) malloc(sizeof(spell_component) * + (result->nodesetval->nodeNr + 1)); + sp->components[result->nodesetval->nodeNr].type = 0; + } + for (component = sp->components, k = 0; k != result->nodesetval->nodeNr; + ++k) { + const resource_type *rtype; + xmlNodePtr node = result->nodesetval->nodeTab[k]; + propValue = xmlGetProp(node, BAD_CAST "name"); + assert(propValue); + rtype = rt_find((const char *)propValue); + if (!rtype) { + log_error("spell %s uses unknown component %s.\n", sp->sname, (const char *)propValue); xmlFree(propValue); - component->amount = xml_ivalue(node, "amount", 1); - component->cost = SPC_FIX; - propValue = xmlGetProp(node, BAD_CAST "cost"); - if (propValue != NULL) { - if (strcmp((const char *)propValue, "linear") == 0) { - component->cost = SPC_LINEAR; - } else if (strcmp((const char *)propValue, "level") == 0) { - component->cost = SPC_LEVEL; - } - xmlFree(propValue); - } - component++; + continue; } - xmlXPathFreeObject(result); - } - - if (valid) { - register_spell(sp); + component->type = rtype; + xmlFree(propValue); + component->amount = xml_ivalue(node, "amount", 1); + component->cost = SPC_FIX; + propValue = xmlGetProp(node, BAD_CAST "cost"); + if (propValue != NULL) { + if (strcmp((const char *)propValue, "linear") == 0) { + component->cost = SPC_LINEAR; + } else if (strcmp((const char *)propValue, "level") == 0) { + component->cost = SPC_LEVEL; + } + xmlFree(propValue); + } + component++; } + xmlXPathFreeObject(result); } } diff --git a/src/tests.c b/src/tests.c index ab345bd88..46e5c4304 100644 --- a/src/tests.c +++ b/src/tests.c @@ -109,6 +109,7 @@ void test_cleanup(void) global.functions.wage = NULL; default_locale = 0; locales = 0; /* TODO: this is evil and leaky */ + free_spells(); /* TODO: this is just as bad! */ free_gamedata(); } diff --git a/src/util/critbit.c b/src/util/critbit.c index bfedfed7b..7474ec64b 100644 --- a/src/util/critbit.c +++ b/src/util/critbit.c @@ -46,6 +46,7 @@ void * make_external_node(const void * key, size_t keylen) ptrdiff_t numvalue = (char *)data - (char*)0; assert((numvalue&1)==0); #endif + assert(keylen); memcpy(data, &keylen, sizeof(size_t)); memcpy(data+sizeof(size_t), key, keylen); return (void*)(data+1); @@ -286,7 +287,7 @@ const void * cb_find(critbit_tree * cb, const void * key, size_t keylen) ptr = node->child[branch]; } from_external_node(ptr, &str, &len); - if (len==keylen && memcmp(key, str, keylen)==0) { + if (len>=keylen && memcmp(key, str, keylen)==0) { return str; } return 0; @@ -330,14 +331,15 @@ int cb_erase(critbit_tree * cb, const void * key, size_t keylen) } } -size_t cb_new_kv(const char *key, void * value, size_t len, void * dst) +size_t cb_new_kv(const char *key, size_t keylen, void * value, size_t len, void * out) { - size_t keylen = strlen(key)+1; + char * dst = (char*)out; if (dst!=key) { memcpy(dst, key, keylen); } - memcpy((char*)dst+keylen, value, len); - return len+keylen; + dst[keylen] = 0; + memcpy(dst+keylen+1, value, len); + return len+keylen+1; } void cb_get_kv(const void *kv, void * value, size_t len) diff --git a/src/util/critbit.h b/src/util/critbit.h index 67534d83f..9ce95599b 100644 --- a/src/util/critbit.h +++ b/src/util/critbit.h @@ -47,7 +47,7 @@ void cb_clear(critbit_tree * cb); #define cb_find_prefix_str(cb, key, results, numresults, offset) \ cb_find_prefix(cb, (void *)key, strlen(key), results, numresults, offset) -size_t cb_new_kv(const char *key, void * value, size_t len, void * dst); +size_t cb_new_kv(const char *key, size_t keylen, void * value, size_t len, void * out); void cb_get_kv(const void *kv, void * value, size_t len); #ifdef __cplusplus diff --git a/src/util/functions.c b/src/util/functions.c index 7e22e7293..a1605cea4 100644 --- a/src/util/functions.c +++ b/src/util/functions.c @@ -45,6 +45,6 @@ void register_function(pf_generic fun, const char *name) size_t len = strlen(name); assert(len #include -#define QL_MAXSIZE 14 /* total struct is 64 bytes */ -#define QL_LIMIT 7 +#define QL_MAXSIZE 14 /* max. number of elements unrolled into one node */ +#define QL_LIMIT 7 /* this many or fewer number in a node => attempt merge */ +/* The total size of this struct is 64 bytes on a 32-bit system with + * normal alignment. YMMV, so on a 64-bit system, twiddle the + * constants above */ struct quicklist { struct quicklist *next; int num_elements; @@ -170,11 +173,11 @@ int ql_advance(struct quicklist **iterator, int *index, int stride) void ql_free(struct quicklist *ql) { - if (!ql) - return; - if (ql->next) - ql_free(ql->next); - free(ql); + while (ql) { + quicklist * qn = ql; + ql = ql->next; + free(qn); + } } int ql_set_remove(struct quicklist **qlp, void *data) diff --git a/src/util/translation.c b/src/util/translation.c index 365b463c1..411b8aee0 100644 --- a/src/util/translation.c +++ b/src/util/translation.c @@ -162,8 +162,8 @@ void add_function(const char *symbol, evalfun parse) size_t len = strlen(symbol); assert(len+1+sizeof(parse)<=sizeof(buffer)); - cb_new_kv(symbol, &parse, sizeof(parse), buffer); - cb_insert(&functions, buffer, len+1+sizeof(parse)); + len = cb_new_kv(symbol, len, &parse, sizeof(parse), buffer); + cb_insert(&functions, buffer, len); } static evalfun find_function(const char *symbol)