Merge pull request #555 from ennorehling/develop

memory leak in addtoken/freetokens

src/direction.c

@@ -12,7 +12,7 @@ void init_direction(const struct locale *lang, direction_t dir, const char *str)
     void **tokens = get_translations(lang, UT_DIRECTIONS);
     variant token;
     token.i = dir;
-    addtoken(tokens, str, token);
+    addtoken((struct tnode **)tokens, str, token);
 }
 
 void init_directions(struct locale *lang) {
@@ -47,7 +47,7 @@ void init_directions(struct locale *lang) {
         if (str) {
             variant token;
             token.i = dirs[i].direction;
-            addtoken(tokens, str, token);
+            addtoken((struct tnode **)tokens, str, token);
         }
     }
 }

src/direction.test.c

@@ -3,6 +3,7 @@
 #include "direction.h"
 #include "tests.h"
 
+#include <util/umlaut.h>
 #include <util/language.h>
 
 #include <CuTest.h>
@@ -10,7 +11,7 @@
 static void test_init_directions(CuTest *tc) {
     struct locale *lang;
 
-    test_cleanup();
+    test_setup();
     lang = get_or_create_locale("de");
     locale_setstring(lang, "dir_nw", "NW");
     init_directions(lang);
@@ -20,19 +21,23 @@ static void test_init_directions(CuTest *tc) {
 
 static void test_init_direction(CuTest *tc) {
     struct locale *lang;
-    test_cleanup();
+    test_setup();
 
     lang = get_or_create_locale("de");
     init_direction(lang, D_NORTHWEST, "NW");
+    init_direction(lang, D_NORTHWEST, "northwest");
     init_direction(lang, D_EAST, "OST");
+    init_direction(lang, D_EAST, "O");
     CuAssertIntEquals(tc, D_NORTHWEST, get_direction("nw", lang));
+    CuAssertIntEquals(tc, D_NORTHWEST, get_direction("northwest", lang));
     CuAssertIntEquals(tc, D_EAST, get_direction("ost", lang));
+    CuAssertIntEquals(tc, D_EAST, get_direction("O", lang));
     CuAssertIntEquals(tc, NODIRECTION, get_direction("east", lang));
     test_cleanup();
 }
 
 static void test_finddirection(CuTest *tc) {
-    test_cleanup();
+    test_setup();
     CuAssertIntEquals(tc, D_SOUTHWEST, finddirection("southwest"));
     CuAssertIntEquals(tc, D_SOUTHEAST, finddirection("southeast"));
     CuAssertIntEquals(tc, D_NORTHWEST, finddirection("northwest"));
@@ -42,6 +47,7 @@ static void test_finddirection(CuTest *tc) {
     CuAssertIntEquals(tc, D_PAUSE, finddirection("pause"));
     CuAssertIntEquals(tc, NODIRECTION, finddirection(""));
     CuAssertIntEquals(tc, NODIRECTION, finddirection("potato"));
+    test_cleanup();
 }
 
 CuSuite *get_direction_suite(void)

src/kernel/build.test.c

@@ -282,7 +282,7 @@ static void test_build_destroy_road(CuTest *tc)
     r = test_create_region(0, 0, 0);
     rsetroad(r, D_EAST, 100);
     u = test_create_unit(f = test_create_faction(0), r);
-    ord = create_order(K_DESTROY, f->locale, "%s %s", LOC(f->locale, parameters[P_ROAD]), LOC(f->locale, directions[D_EAST]));
+    u->orders = ord = create_order(K_DESTROY, f->locale, "%s %s", LOC(f->locale, parameters[P_ROAD]), LOC(f->locale, directions[D_EAST]));
 
     CuAssertIntEquals(tc, 0, destroy_cmd(u, ord));
     CuAssertIntEquals(tc, 100, rroad(r, D_EAST));
@@ -336,7 +336,7 @@ static void test_build_destroy_road_guard(CuTest *tc)
     rsetroad(r, D_EAST, 100);
     ug = test_create_guard(r, 0, 0);
     u = test_create_unit(f = test_create_faction(0), r);
-    ord = create_order(K_DESTROY, f->locale, "%s %s", LOC(f->locale, parameters[P_ROAD]), LOC(f->locale, directions[D_EAST]));
+    u->orders = ord = create_order(K_DESTROY, f->locale, "%s %s", LOC(f->locale, parameters[P_ROAD]), LOC(f->locale, directions[D_EAST]));
 
     set_level(u, SK_ROAD_BUILDING, 1);
     CuAssertIntEquals(tc, 0, destroy_cmd(u, ord));
@@ -367,7 +367,7 @@ static void test_build_destroy_road_limit(CuTest *tc)
     r = test_create_region(0, 0, 0);
     rsetroad(r, D_EAST, 100);
     u = test_create_unit(f = test_create_faction(0), r);
-    ord = create_order(K_DESTROY, f->locale, "1 %s %s", LOC(f->locale, parameters[P_ROAD]), LOC(f->locale, directions[D_EAST]));
+    u->orders = ord = create_order(K_DESTROY, f->locale, "1 %s %s", LOC(f->locale, parameters[P_ROAD]), LOC(f->locale, directions[D_EAST]));
 
     set_level(u, SK_ROAD_BUILDING, 1);
     CuAssertIntEquals(tc, 0, destroy_cmd(u, ord));

src/kernel/building.c

@@ -311,7 +311,7 @@ const building_type *findbuildingtype(const char *name,
 
             const char *n = LOC(lang, btype->_name);
             type.v = (void *)btype;
-            addtoken(&bn->names, n, type);
+            addtoken((struct tnode **)&bn->names, n, type);
         }
         bnames = bn;
     }

src/kernel/command.c

@@ -78,7 +78,7 @@ const char *str, parser fun)
     cmd->fun = fun;
     cmd->nodes = tnext;
     var.v = cmd;
-    addtoken(keys, str, var);
+    addtoken((struct tnode **)keys, str, var);
 }
 
 static int do_command_i(const void *keys, struct unit *u, struct order *ord)

src/kernel/config.c

@@ -471,7 +471,7 @@ void init_terrains_translation(const struct locale *lang) {
         var.v = (void *)terrain;
         name = locale_string(lang, terrain->_name, false);
         if (name) {
-            addtoken(tokens, name, var);
+            addtoken((struct tnode **)tokens, name, var);
         }
         else {
             log_debug("no translation for terrain %s in locale %s", terrain->_name, locale_name(lang));
@@ -490,7 +490,7 @@ void init_options_translation(const struct locale * lang) {
         if (options[i]) {
             const char *name = locale_string(lang, options[i], false);
             if (name) {
-                addtoken(tokens, name, var);
+                addtoken((struct tnode **)tokens, name, var);
             }
             else {
                 log_debug("no translation for OPTION %s in locale %s", options[i], locale_name(lang));
@@ -525,7 +525,7 @@ void init_locale(struct locale *lang)
             var.i = i;
             name = LOC(lang, mkname("school", tok));
             if (name) {
-                addtoken(tokens, name, var);
+                addtoken((struct tnode **)tokens, name, var);
             }
             else {
                 log_warning("no translation for magic school %s in locale %s", tok, locale_name(lang));
@@ -544,9 +544,9 @@ void init_locale(struct locale *lang)
         const char *name;
         var.v = (void *)rc;
         name = locale_string(lang, rc_name_s(rc, NAME_PLURAL), false);
-        if (name) addtoken(tokens, name, var);
+        if (name) addtoken((struct tnode **)tokens, name, var);
         name = locale_string(lang, rc_name_s(rc, NAME_SINGULAR), false);
-        if (name) addtoken(tokens, name, var);
+        if (name) addtoken((struct tnode **)tokens, name, var);
     }
 
     init_parameters(lang);

src/kernel/ship.c

@@ -78,7 +78,7 @@ const ship_type *findshiptype(const char *name, const struct locale *lang)
             variant var2;
             const char *n = LOC(lang, stype->_name);
             var2.v = (void *)stype;
-            addtoken(&sn->names, n, var2);
+            addtoken((struct tnode **)&sn->names, n, var2);
         }
         snames = sn;
     }

src/laws.c

@@ -1438,8 +1438,8 @@ static void init_prefixnames(void)
                     LOC(lang, mkname("prefix", race_prefixes[key]));
                 if (findtoken(in->names, pname, &var) == E_TOK_NOMATCH || var.i != key) {
                     var.i = key;
-                    addtoken(&in->names, pname, var);
-                    addtoken(&in->names, LOC(lang, mkname("prefix",
+                    addtoken((struct tnode **)&in->names, pname, var);
+                    addtoken((struct tnode **)&in->names, LOC(lang, mkname("prefix",
                         race_prefixes[key])), var);
                 }
             }

src/magic.c

@@ -2954,7 +2954,7 @@ static void select_spellbook(void **tokens, spellbook *sb, const struct locale *
         else {
             variant token;
             token.v = sp;
-            addtoken(tokens, n, token);
+            addtoken((struct tnode **)tokens, n, token);
         }
     }
 }

src/piracy.c

@@ -209,6 +209,7 @@ void piracy_cmd(unit * u)
     /* Bewegung ausführen */
     init_order(ord);
     move_cmd(u, ord);
+    free_order(ord);
 }
 
 void age_piracy(region *r) {

src/seen.test.c

@@ -23,7 +23,7 @@ static void test_add_seen(CuTest *tc) {
     region *r;
     seen_region **seen, *sr;
 
-    test_cleanup();
+    test_setup();
     seen = seen_init();
     r = test_create_region(0, 0, 0);
     sr = add_seen(seen, r, see_travel, false);
@@ -47,7 +47,7 @@ static void test_faction_add_seen(CuTest *tc) {
     faction *f;
     seen_region *sr;
 
-    test_cleanup();
+    test_setup();
     f = test_create_faction(0);
     f->seen = seen_init();
     test_create_region(0, 0, 0);
@@ -65,7 +65,7 @@ static void test_prepare_seen(CuTest *tc) {
     faction *f;
     unit *u;
 
-    test_cleanup();
+    test_setup();
     f = test_create_faction(0);
     r = test_create_region(0, 0, 0);
     u = test_create_unit(f, r);
@@ -90,7 +90,7 @@ static void test_seen_travelthru(CuTest *tc) {
     faction *f;
     unit *u;
 
-    test_cleanup();
+    test_setup();
     setup_seen(0, 0);
     r = test_create_region(0, 0, 0);
     f = test_create_faction(0);
@@ -116,7 +116,7 @@ static void test_seen_region(CuTest *tc) {
     seen_region **seen, *sr;
     region *r;
 
-    test_cleanup();
+    test_setup();
     setup_seen(0, 0);
     r = test_create_region(0, 0, 0);
     seen = seen_init();
@@ -131,7 +131,7 @@ static void test_seen_interval_backward(CuTest *tc) {
     region *r, *first, *last;
     seen_region **seen;
 
-    test_cleanup();
+    test_setup();
     r = test_create_region(0, 0, 0);
     setup_seen(0, 0);
     seen = seen_init();
@@ -150,7 +150,7 @@ static void test_seen_interval_forward(CuTest *tc) {
     region *r, *first, *last;
     seen_region **seen;
 
-    test_cleanup();
+    test_setup();
     setup_seen(0, 0);
     r = test_create_region(0, 0, 0);
     seen = seen_init();
@@ -175,7 +175,7 @@ static void test_seenhash_map(CuTest *tc) {
     seen_region **seen;
     int i = 0;
 
-    test_cleanup();
+    test_setup();
     seen = seen_init();
     r = test_create_region(1, 1, 0);
     add_seen(seen, r, see_unit, false);

src/test_eressea.c

@@ -76,6 +76,7 @@ int RunAllTests(int argc, char *argv[])
     ADD_SUITE(bsdstring);
     ADD_SUITE(functions);
     ADD_SUITE(gamedata);
+    ADD_SUITE(language);
     ADD_SUITE(parser);
     ADD_SUITE(password);
     ADD_SUITE(umlaut);

src/util/CMakeLists.txt

@@ -11,7 +11,7 @@ bsdstring.test.c
 functions.test.c
 gamedata.test.c
 # goodies.test.c
-# language.test.c
+language.test.c
 # lists.test.c
 # log.test.c
 # message.test.c

src/util/language.test.c

@@ -0,0 +1,25 @@
+#include <platform.h>
+#include <config.h>
+#include "language.h"
+
+#include <CuTest.h>
+#include <tests.h>
+
+extern const char *directions[];
+
+static void test_language(CuTest *tc)
+{
+    const char *str;
+    test_setup();
+    default_locale = test_create_locale();
+    str = directions[1];
+    CuAssertStrEquals(tc, str, locale_getstring(default_locale, str));
+    test_cleanup();
+}
+
+CuSuite *get_language_suite(void)
+{
+    CuSuite *suite = CuSuiteNew();
+    SUITE_ADD_TEST(suite, test_language);
+    return suite;
+}

src/util/umlaut.c

@@ -32,7 +32,7 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 typedef struct tref {
     struct tref *nexthash;
     ucs4_t ucs;
-    void *node;
+    struct tnode *node;
 } tref;
 
 #define LEAF 1                  /* leaf node for a word. always matches */
@@ -42,6 +42,7 @@ typedef struct tnode {
     struct tref *next[NODEHASHSIZE];
     unsigned char flags;
     variant id;
+    int refcount;
 } tnode;
 
 char * transliterate(char * out, size_t size, const char * in)
@@ -114,7 +115,13 @@ char * transliterate(char * out, size_t size, const char * in)
     return *src ? 0 : out;
 }
 
-void addtoken(void ** root, const char *str, variant id)
+tnode * mknode(void) {
+    tnode * node = calloc(1, sizeof(tnode));
+    node->refcount = 1;
+    return node;
+}
+
+void addtoken(tnode ** root, const char *str, variant id)
 {
     tnode * tk;
     static const struct replace {
@@ -128,7 +135,7 @@ void addtoken(void ** root, const char *str, variant id)
 
     assert(root && str);
     if (!*root) {
-        tk = *root = calloc(1, sizeof(tnode));
+        tk = *root = mknode();
     }
     else {
         tk = *root;
@@ -161,7 +168,7 @@ void addtoken(void ** root, const char *str, variant id)
             next = next->nexthash;
         if (!next) {
             tref *ref;
-            tnode *node = (tnode *)calloc(1, sizeof(tnode)); // TODO: what is the reason for this empty node to exist?
+            tnode *node = mknode(); // TODO: what is the reason for this empty node to exist?
 
             if (ucs < 'a' || ucs > 'z') {
                 lcs = towlower((wint_t)ucs);
@@ -187,6 +194,7 @@ void addtoken(void ** root, const char *str, variant id)
                 assert_alloc(ref);
                 ref->ucs = lcs;
                 ref->node = node;
+                ++node->refcount;
                 ref->nexthash = tk->next[index];
                 tk->next[index] = ref;
             }
@@ -212,23 +220,25 @@ void addtoken(void ** root, const char *str, variant id)
     }
 }
 
-void freetokens(void * root)
+void freetokens(tnode * root)
 {
-    tnode * node = (tnode *)root;
+    tnode * node = root;
     int i;
     for (i = 0; node && i != NODEHASHSIZE; ++i) {
         if (node->next[i]) {
-            tref ** refs = &node->next[i];
-            freetokens(node->next[i]->node);
-            while (*refs) {
-                tref * ref = *refs;
-                *refs = ref->nexthash;
-//                free(ref->node);
+            tref * ref = node->next[i];
+            while (ref) {
+                tref * next = ref->nexthash;
+                freetokens(ref->node);
                 free(ref);
+                ref = next;
             }
+            node->next[i] = 0;
         }
     }
-    free(node);
+    if (--node->refcount == 0) {
+        free(node);
+    }
 }
 
 int findtoken(const void * root, const char *key, variant * result)

src/util/umlaut.h

@@ -29,10 +29,11 @@ extern "C" {
 #define E_TOK_NOMATCH (-1)
 #define E_TOK_SUCCESS 0
 #define NODEHASHSIZE 8
+    struct tnode;
 
     int findtoken(const void *tk, const char *str, variant * result);
-    void addtoken(void **root, const char *str, variant id);
-    void freetokens(void *root);
+    void addtoken(struct tnode **root, const char *str, variant id);
+    void freetokens(struct tnode *root);
 
     char * transliterate(char * out, size_t size, const char * in);
 

src/util/umlaut.test.c

@@ -28,7 +28,7 @@ static void test_transliterate(CuTest * tc)
 
 static void test_transliterations(CuTest *tc) {
     const char * umlauts = "\xc3\xa4\xc3\xb6\xc3\xbc\xc3\x9f"; /* auml ouml uuml szlig nul */
-    void * tokens = 0;
+    struct tnode * tokens = 0;
     variant id;
     int result;
 
@@ -48,7 +48,7 @@ static void test_transliterations(CuTest *tc) {
 
 static void test_directions(CuTest * tc)
 {
-    void * tokens = 0;
+    struct tnode * tokens = 0;
     variant id;
     int result;
 
@@ -64,7 +64,7 @@ static void test_directions(CuTest * tc)
 
 static void test_umlaut(CuTest * tc)
 {
-    void * tokens = 0;
+    struct tnode *tokens = 0;
     variant id;
     int result;
 
@@ -97,6 +97,17 @@ static void test_umlaut(CuTest * tc)
     freetokens(tokens);
 }
 
+static void test_leak(CuTest *tc) {
+    struct tnode *tokens = NULL;
+    variant token;
+    
+    token.i = 42;
+    addtoken(&tokens, "NO", token);
+    addtoken(&tokens, "nw", token);
+    CuAssertIntEquals(tc, E_TOK_SUCCESS, findtoken(tokens, "n", &token));
+    freetokens(tokens);
+}
+
 CuSuite *get_umlaut_suite(void)
 {
     CuSuite *suite = CuSuiteNew();
@@ -104,5 +115,6 @@ CuSuite *get_umlaut_suite(void)
     SUITE_ADD_TEST(suite, test_directions);
     SUITE_ADD_TEST(suite, test_transliterate);
     SUITE_ADD_TEST(suite, test_transliterations);
+    SUITE_ADD_TEST(suite, test_leak);
     return suite;
 }

src/vortex.c

@@ -36,7 +36,7 @@ void register_special_direction(struct locale *lang, const char *name)
         char *str = _strdup(name);
 
         var.v = str;
-        addtoken(tokens, token, var);
+        addtoken((struct tnode **)tokens, token, var);
 
         if (lang == locales) {
             dir_lookup *dl = malloc(sizeof(dir_lookup));