Merge pull request #43 from badgerman/master

Fix FFL_NOTIMEOUT related crash in E2

src/kernel/curse.c

@@ -1,5 +1,5 @@
 /*
-Copyright (c) 1998-2010, Enno Rehling <enno@eressea.de>
+Copyright (c) 1998-2014, Enno Rehling <enno@eressea.de>
 Katja Zedel <katze@felidae.kn-bremen.de
 Christian Schlittchen <corwin@amber.kn-bremen.de>
 
@@ -126,7 +126,8 @@ int curse_age(attrib * a)
     }
     if (result != 0) {
         c->duration = 0;
-  } else if (c->duration != INT_MAX) {
+    }
+    else if (c->duration != INT_MAX) {
         c->duration = _max(0, c->duration - 1);
     }
     return c->duration;
@@ -198,19 +199,22 @@ int curse_read(attrib * a, void *owner, struct storage *store)
     if (global.data_version >= CURSEVIGOURISFLOAT_VERSION) {
         READ_FLT(store, &flt);
         c->vigour = flt;
-  } else {
+    }
+    else {
         READ_INT(store, &n);
         c->vigour = (float)n;
     }
     if (global.data_version < INTPAK_VERSION) {
         ur = read_reference(&c->magician, store, read_int, resolve_unit);
-  } else {
+    }
+    else {
         ur = read_reference(&c->magician, store, read_unit_reference, resolve_unit);
     }
     if (global.data_version < CURSEFLOAT_VERSION) {
         READ_INT(store, &n);
         c->effect = (float)n;
-  } else {
+    }
+    else {
         READ_FLT(store, &flt);
         c->effect = flt;
     }
@@ -225,7 +229,8 @@ int curse_read(attrib * a, void *owner, struct storage *store)
     }
     if (global.data_version < CURSEFLAGS_VERSION) {
         c_setflag(c, flags);
-  } else {
+    }
+    else {
         c->flags = flags;
     }
     c_clearflag(c, CURSE_ISNEW);
@@ -312,7 +317,8 @@ const curse_type *ct_find(const char *c)
 
         if (strcmp(c, type->cname) == 0) {
             return type;
-    } else {
+        }
+        else {
             size_t k = _min(strlen(c), strlen(type->cname));
             if (!_memicmp(c, type->cname, k)) {
                 return type;
@@ -327,7 +333,7 @@ const curse_type *ct_find(const char *c)
  * einen pointer auf die struct zurück.
  */
 
-bool cmp_curse(const attrib * a, const void *data)
+static bool cmp_curse(const attrib * a, const void *data)
 {
     const curse *c = (const curse *)data;
     if (a->type->flags & ATF_CURSE) {
@@ -337,29 +343,6 @@ bool cmp_curse(const attrib * a, const void *data)
     return false;
 }
 
-bool cmp_cursetype(const attrib * a, const void *data)
-{
-  const curse_type *ct = (const curse_type *)data;
-  if (a->type->flags & ATF_CURSE) {
-    if (!data || ct == ((curse *) a->data.v)->type)
-      return true;
-  }
-  return false;
-}
-
-curse *get_cursex(attrib * ap, const curse_type * ctype, variant data,
-  bool(*compare) (const curse *, variant))
-{
-  attrib *a = a_select(ap, ctype, cmp_cursetype);
-  while (a) {
-    curse *c = (curse *) a->data.v;
-    if (compare(c, data))
-      return c;
-    a = a_select(a->next, ctype, cmp_cursetype);
-  }
-  return NULL;
-}
-
 curse *get_curse(attrib * ap, const curse_type * ctype)
 {
     attrib *a = ap;
@@ -372,7 +355,8 @@ curse *get_curse(attrib * ap, const curse_type * ctype)
                     return c;
                 a = a->next;
             }
-    } else {
+        }
+        else {
             a = a->nexttype;
         }
     }
@@ -388,11 +372,10 @@ curse *findcurse(int cid)
 }
 
 /* ------------------------------------------------------------- */
-void remove_curse(attrib ** ap, const curse * c)
+bool remove_curse(attrib ** ap, const curse * c)
 {
     attrib *a = a_select(*ap, c, cmp_curse);
-  if (a)
+    return a && a_remove(ap, a) == 1;
-    a_remove(ap, a);
 }
 
 /* gibt die allgemeine Stärke der Verzauberung zurück. id2 wird wie
@@ -423,7 +406,8 @@ float curse_changevigour(attrib ** ap, curse * c, float vigour)
     if (vigour <= 0) {
         remove_curse(ap, c);
         vigour = 0;
-  } else {
+    }
+    else {
         set_cursevigour(c, vigour);
     }
     return vigour;
@@ -576,7 +560,8 @@ curse *create_curse(unit * magician, attrib ** ap, const curse_type * ct,
             }
         }
         set_curseingmagician(magician, *ap, ct);
-  } else {
+    }
+    else {
         c = make_curse(magician, ap, ct, vigour, duration, effect, men);
     }
     return c;
@@ -798,16 +783,19 @@ float destr_curse(curse * c, int cast_level, float force)
             force -= c->vigour;
             if (c->type->change_vigour) {
                 c->type->change_vigour(c, -((float)cast_level + 1) / 2);
-      } else {
+            }
+            else {
                 c->vigour -= cast_level + 1 / 2;
             }
         }
-  } else {                      /* Zauber ist stärker als curse */
+    }
+    else {                      /* Zauber ist stärker als curse */
         if (force >= c->vigour) {   /* reicht die Kraft noch aus? */
             force -= c->vigour;
             if (c->type->change_vigour) {
                 c->type->change_vigour(c, -c->vigour);
-      } else {
+            }
+            else {
                 c->vigour = 0;
             }
 

src/kernel/curse.h

@@ -1,5 +1,5 @@
 /*
-Copyright (c) 1998-2010, Enno Rehling <enno@eressea.de>
+Copyright (c) 1998-2014, Enno Rehling <enno@eressea.de>
                          Katja Zedel <katze@felidae.kn-bremen.de
                          Christian Schlittchen <corwin@amber.kn-bremen.de>
 
@@ -251,7 +251,7 @@ extern "C" {
   bool is_cursed_internal(struct attrib *ap, const curse_type * ctype);
   /* ignoriert CURSE_ISNEW */
 
-  extern void remove_curse(struct attrib **ap, const struct curse *c);
+  bool remove_curse(struct attrib **ap, const struct curse *c);
   /* löscht einen konkreten Spruch auf einem Objekt.
    */
 
@@ -280,11 +280,6 @@ extern "C" {
    * unterschiedlich gewünscht sein
    * */
 
-  extern struct curse *get_cursex(struct attrib *ap, const curse_type * ctype,
-    variant data, bool(*compare) (const struct curse *, variant));
-  /* gibt pointer auf die erste curse-struct zurück, deren Typ ctype ist,
-   * und für die compare() true liefert, oder einen NULL-pointer.
-   * */
   extern struct curse *get_curse(struct attrib *ap, const curse_type * ctype);
   /* gibt pointer auf die erste curse-struct zurück, deren Typ ctype ist,
    * oder einen NULL-pointer
@@ -303,9 +298,6 @@ extern "C" {
   extern void curse_done(struct attrib *a);
   extern int curse_age(struct attrib *a);
 
-  extern bool cmp_curse(const struct attrib *a, const void *data);
-  extern bool cmp_cursetype(const struct attrib *a, const void *data);
-
   extern float destr_curse(struct curse *c, int cast_level, float force);
 
   extern int resolve_curse(variant data, void *address);

src/kernel/faction.test.c

@@ -1,5 +1,6 @@
 #include <platform.h>
 
+#include <kernel/ally.h>
 #include <kernel/faction.h>
 #include <kernel/types.h>
 #include <kernel/race.h>
@@ -13,6 +14,21 @@
 #include <assert.h>
 #include <stdio.h>
 
+static void test_remove_empty_factions_allies(CuTest *tc) {
+    faction *f1, *f2;
+    region *r;
+
+    test_cleanup();
+    r = test_create_region(0, 0, 0);
+    f1 = test_create_faction(0);
+    test_create_unit(f1, r);
+    f2 = test_create_faction(0);
+    ally_add(&f1->allies, f2);
+    remove_empty_factions();
+    CuAssertPtrEquals(tc, 0, f1->allies);
+    test_cleanup();
+}
+
 static void test_remove_empty_factions(CuTest *tc) {
     faction *f, *fm;
     int fno;
@@ -95,6 +111,7 @@ CuSuite *get_faction_suite(void)
     CuSuite *suite = CuSuiteNew();
     SUITE_ADD_TEST(suite, test_addfaction);
     SUITE_ADD_TEST(suite, test_remove_empty_factions);
+    SUITE_ADD_TEST(suite, test_remove_empty_factions_allies);
     SUITE_ADD_TEST(suite, test_remove_dead_factions);
     SUITE_ADD_TEST(suite, test_get_monsters);
     return suite;

src/kernel/save.c

@@ -113,8 +113,6 @@ char *rns(FILE * f, char *c, size_t size)
     return c;
 }
 
-extern unsigned int __at_hashkey(const char *s);
-
 
 static unit *unitorders(FILE * F, int enc, struct faction *f)
 {
@@ -1315,7 +1313,6 @@ faction *readfaction(struct gamedata * data)
             f->flags = FFL_NPC | FFL_NOIDLEOUT;
         }
     }
-    assert((f->flags&FFL_SAVEMASK) == f->flags);
 
     a_read(data->store, &f->attribs, f);
     if (data->version >= CLAIM_VERSION) {

src/modules/gmcmd.c

@@ -35,16 +35,6 @@
 
 /* util includes */
 #include <util/attrib.h>
-#include <util/base36.h>
-#include <util/event.h>
-#include <util/goodies.h>
-#include <util/language.h>
-#include <util/lists.h>
-#include <util/log.h>
-#include <util/umlaut.h>
-#include <util/parser.h>
-#include <util/rng.h>
-
 #include <storage.h>
 
 /* libc includes */
@@ -56,18 +46,11 @@
 static int read_permissions(attrib * a, void *owner, struct storage *store)
 {
   attrib *attr = NULL;
-  a_read(store, &attr, NULL);
+  a_read(store, &attr, owner);
-  a_free(attr);
+  a_remove(&attr, a);
   return AT_READ_OK;
 }
 
-struct attrib_type at_permissions = {
-  "GM:permissions",
-  NULL, NULL, NULL,
-  NULL, read_permissions,
-  ATF_UNIQUE
-};
-
 static int read_gmcreate(attrib * a, void *owner, struct storage *store)
 {
   char zText[32];
@@ -75,15 +58,8 @@ static int read_gmcreate(attrib * a, void *owner, struct storage *store)
   return AT_READ_OK;
 }
 
-/* at_gmcreate specifies that the owner can create items of a particular type */
-attrib_type at_gmcreate = {
-  "GM:create",
-  NULL, NULL, NULL,
-  NULL, read_gmcreate
-};
-
 void register_gmcmd(void)
 {
-  at_register(&at_gmcreate);
+    at_deprecate("GM:create", read_gmcreate);
-  at_register(&at_permissions);
+    at_deprecate("GM:permissions", read_permissions);
 }

src/test_eressea.c

@@ -30,6 +30,7 @@ int RunAllTests(void)
   ADD_TESTS(suite, race);
   /* util */
   ADD_TESTS(suite, config);
+  ADD_TESTS(suite, attrib);
   ADD_TESTS(suite, base36);
   ADD_TESTS(suite, bsdstring);
   ADD_TESTS(suite, functions);

src/tests.h

@@ -1,9 +1,22 @@
 #ifndef ERESSEA_TESTS_H
 #define ERESSEA_TESTS_H
+
+#include <stdlib.h>
+
 #ifdef __cplusplus
 extern "C" {
 #endif
 
+    struct region;
+    struct unit;
+    struct faction;
+    struct building;
+    struct ship;
+    struct item_type;
+    struct building_type;
+    struct ship_type;
+    struct terrain_type;
+
     void test_cleanup(void);
 
     struct terrain_type * test_create_terrain(const char * name, unsigned int flags);

src/triggers/removecurse.c

@@ -1,5 +1,5 @@
 /*
-Copyright (c) 1998-2010, Enno Rehling <enno@eressea.de>
+Copyright (c) 1998-2014, Enno Rehling <enno@eressea.de>
 Katja Zedel <katze@felidae.kn-bremen.de
 Christian Schlittchen <corwin@amber.kn-bremen.de>
 
@@ -62,12 +62,10 @@ static int removecurse_handle(trigger * t, void *data)
      */
     removecurse_data *td = (removecurse_data *)t->data.v;
     if (td->curse && td->target) {
-    attrib *a = a_select(td->target->attribs, td->curse, cmp_curse);
+        if (!remove_curse(&td->target->attribs, td->curse)) {
-    if (a) {
-      a_remove(&td->target->attribs, a);
-    } else
             log_error("could not perform removecurse::handle()\n");
         }
+    }
     unused_arg(data);
     return 0;
 }

src/util/CMakeLists.txt

@@ -2,6 +2,7 @@ project(util C)
 
 SET(_TEST_FILES
 base36.test.c
+attrib.test.c
 strings.test.c
 bsdstring.test.c
 functions.test.c

src/util/attrib.c

@@ -29,7 +29,7 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 #include <stdlib.h>
 
 #define MAXATHASH 61
-attrib_type *at_hash[MAXATHASH];
+static attrib_type *at_hash[MAXATHASH];
 
 static unsigned int __at_hashkey(const char *s)
 {
@@ -51,12 +51,14 @@ void at_register(attrib_type * at)
     }
     at->hashkey = __at_hashkey(at->name);
     find = at_hash[at->hashkey % MAXATHASH];
-  while (find && at->hashkey != find->hashkey)
+    while (find && at->hashkey != find->hashkey) {
         find = find->nexthash;
+    }
     if (find && find == at) {
         log_warning("attribute '%s' was registered more than once\n", at->name);
         return;
-  } else {
+    }
+    else {
         assert(!find || !"hashkey is already in use");
     }
     at->nexthash = at_hash[at->hashkey % MAXATHASH];
@@ -149,14 +151,6 @@ attrib *a_add(attrib ** pa, attrib * a)
     return a;
 }
 
-void a_free(attrib * a)
-{
-  const attrib_type *at = a->type;
-  if (at->finalize)
-    at->finalize(a);
-  free(a);
-}
-
 static int a_unlink(attrib ** pa, attrib * a)
 {
     attrib **pnexttype = pa;
@@ -180,7 +174,8 @@ static int a_unlink(attrib ** pa, attrib * a)
                 return 1;
             while (*pnext && (*pnext)->type != a->type)
                 pnext = &(*pnext)->next;
-    } else {
+        }
+        else {
             pnext = &(*pnexttype)->next;
         }
         while (*pnext && (*pnext)->type == a->type) {
@@ -194,6 +189,14 @@ static int a_unlink(attrib ** pa, attrib * a)
     return 0;
 }
 
+static void a_free(attrib * a)
+{
+    const attrib_type *at = a->type;
+    if (at->finalize)
+        at->finalize(a);
+    free(a);
+}
+
 int a_remove(attrib ** pa, attrib * a)
 {
     int ok;
@@ -294,12 +297,14 @@ int a_read(struct storage *store, attrib ** attribs, void *owner)
         if (at) {
             reader = at->read;
             na = a_new(at);
-    } else {
+        }
+        else {
             const void * kv;
             cb_find_prefix(&cb_deprecated, zText, strlen(zText) + 1, &kv, 1, 0);
             if (kv) {
                 cb_get_kv(kv, &reader, sizeof(reader));
-      } else {
+            }
+            else {
                 fprintf(stderr, "attribute hash: %d (%s)\n", key, zText);
                 assert(at || !"attribute not registered");
             }
@@ -320,7 +325,8 @@ int a_read(struct storage *store, attrib ** attribs, void *owner)
                     break;
                 }
             }
-    } else {
+        }
+        else {
             assert(!"error: no registered callback can read attribute");
         }
 
@@ -342,7 +348,8 @@ void a_write(struct storage *store, const attrib * attribs, const void *owner)
             WRITE_TOK(store, na->type->name);
             na->type->write(na, owner, store);
             na = na->next;
-    } else {
+        }
+        else {
             na = na->nexttype;
         }
     }

src/util/attrib.h

@@ -18,6 +18,7 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
 #ifndef ATTRIB_H
 #define ATTRIB_H
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -72,7 +73,6 @@ extern "C" {
     extern int a_remove(attrib ** pa, attrib * at);
     extern void a_removeall(attrib ** a, const attrib_type * at);
     extern attrib *a_new(const attrib_type * at);
-  extern void a_free(attrib * a);
 
     extern int a_age(attrib ** attribs);
     extern int a_read(struct storage *store, attrib ** attribs, void *owner);

src/util/attrib.test.c

@@ -0,0 +1,84 @@
+#include <platform.h>
+#include "attrib.h"
+
+#include <CuTest.h>
+#include <tests.h>
+
+static void test_attrib_new(CuTest * tc)
+{
+    attrib_type at_test = { "test" };
+    attrib * a;
+    CuAssertPtrNotNull(tc, (a = a_new(&at_test)));
+    CuAssertPtrEquals(tc, 0, a->next);
+    CuAssertPtrEquals(tc, 0, a->nexttype);
+    CuAssertPtrEquals(tc, (void *)a->type, (void *)&at_test);
+    a_remove(&a, a);
+    CuAssertPtrEquals(tc, 0, a);
+}
+
+
+static void test_attrib_add(CuTest * tc)
+{
+    attrib_type at_foo = { "foo" };
+    attrib_type at_bar = { "bar" };
+    attrib *a, *alist = 0;
+
+    CuAssertPtrNotNull(tc, (a = a_new(&at_foo)));
+    CuAssertPtrEquals(tc, a, a_add(&alist, a));
+    CuAssertPtrEquals(tc, a, alist);
+
+    CuAssertPtrNotNull(tc, (a = a_add(&alist, a_new(&at_foo))));
+    CuAssertPtrEquals_Msg(tc, "new attribute not added after existing", alist->next, a);
+
+    CuAssertPtrNotNull(tc, (a = a_add(&alist, a_new(&at_bar))));
+    CuAssertPtrEquals_Msg(tc, "new atribute not added at end of list", alist->next->next, a);
+
+    CuAssertPtrNotNull(tc, (a = a_add(&alist, a_new(&at_foo))));
+    CuAssertPtrEquals_Msg(tc, "messages not sorted by type", alist->next->next, a);
+    a_removeall(&alist, &at_foo);
+    a_removeall(&alist, &at_bar);
+}
+
+static void test_attrib_remove(CuTest * tc)
+{
+    attrib_type at_foo = { "foo" };
+    attrib *a, *alist = 0;
+
+    CuAssertPtrNotNull(tc, a_add(&alist, a_new(&at_foo)));
+    CuAssertPtrNotNull(tc, a = a_add(&alist, a_new(&at_foo)));
+    CuAssertIntEquals(tc, 1, a_remove(&alist, a));
+    CuAssertPtrNotNull(tc, alist);
+    CuAssertIntEquals(tc, 1, a_remove(&alist, alist));
+    CuAssertPtrEquals(tc, 0, alist);
+}
+
+static void test_attrib_nexttype(CuTest * tc)
+{
+    attrib_type at_foo = { "foo" };
+    attrib_type at_bar = { "bar" };
+    attrib *a, *alist = 0;
+    CuAssertPtrNotNull(tc, (a = a_new(&at_foo)));
+    CuAssertPtrEquals(tc, 0, a->nexttype);
+    CuAssertPtrEquals(tc, a, a_add(&alist, a));
+    CuAssertPtrEquals(tc, 0, alist->nexttype);
+
+    CuAssertPtrNotNull(tc, a_add(&alist, a_new(&at_foo)));
+    CuAssertPtrEquals(tc, 0, alist->nexttype);
+
+    CuAssertPtrNotNull(tc, (a = a_add(&alist, a_new(&at_bar))));
+    CuAssertPtrEquals(tc, a, alist->nexttype);
+    CuAssertPtrEquals(tc, 0, a->nexttype);
+
+    a_removeall(&alist, &at_foo);
+    a_removeall(&alist, &at_bar);
+}
+
+CuSuite *get_attrib_suite(void)
+{
+    CuSuite *suite = CuSuiteNew();
+    SUITE_ADD_TEST(suite, test_attrib_new);
+    SUITE_ADD_TEST(suite, test_attrib_add);
+    SUITE_ADD_TEST(suite, test_attrib_remove);
+    SUITE_ADD_TEST(suite, test_attrib_nexttype);
+    return suite;
+}