Merge pull request #568 from ennorehling/develop

change how dead units' factions are found

s/coverity

@@ -1,9 +1,7 @@
 #!/bin/bash
-set -e
-VERSION="$1"
+#set -e
+VERSION=$(git describe --tags --match 'v*.*.*')
 echo "submitting version $VERSION"
-shift
-DESC="$*"
 cd Debug
 make clean
 ../../coverity/bin/cov-build --dir cov-int make eressea
@@ -12,5 +10,5 @@ curl --form token=IISXKH3A1ngZGfFmBz_aSA \
   --form email=enno.rehling@gmail.com \
   --form file=@eressea.tgz \
   --form version="$VERSION" \
-  --form description="$DESC" \
+  --form description="Eressea coverity check" \
   https://scan.coverity.com/builds?project=eressea%2Fserver

scripts/eressea/autoseed.lua

@@ -39,13 +39,17 @@ local function read_players()
 --    return {{ email = "noreply@mailinator.com", race = "dwarf",  lang = "de" }}
     local players =  {}
     local input = io.open("newfactions", "r")
-    while input do
+    if input then
         local str = input:read("*line")
-        if str==nil then break end
-        local email, race, lang = str:match("([^ ]*) ([^ ]*) ([^ ]*)")
-        if email and string.char(string.byte(email, 1))~='#' then
-            table.insert(players, { race = race, lang = lang, email = email })
+        while str do
+            if str==nil then break end
+            local email, race, lang = str:match("([^ ]*) ([^ ]*) ([^ ]*)")
+            if email and string.char(string.byte(email, 1))~='#' then
+                table.insert(players, { race = race, lang = lang, email = email })
+            end
+            str = input:read("*line")
         end
+        input:close()
     end
     return players
 end

scripts/eressea/cursed.lua

@@ -23,6 +23,7 @@ function cursed.init()
     if f then
         print("found cursed.txt")
         curse(f)
+        f:close()
     end
 end
 

scripts/tools/build-e3.lua

@@ -124,30 +124,33 @@ end
 
 function seed()
   local input = io.open(config.basepath .. "/parteien.txt")
-  for f in factions() do
-    if f.race=="vampunicorn" then
-      local str = input:read("*line")
-      if str==nil then break end
-      local race, lang, email = str:match("([^ ]*) ([^ ]*) ([^ ]*)")
-      f.race = race:lower()
-      f.options = f.options + 4096
-      f.email = email
-      f.locale = lang
-      for u in f.units do
-        u.race = race:lower()
-        u.hp = u.hp_max
-        local b = building.create(u.region, "castle")
-        if lang=="de" then
-          u.name = "Entdecker"
-          b.name = "Heimat"
-        else
-          u.name = "Explorer"
-          b.name = "Home"
+  if input then
+      for f in factions() do
+        if f.race=="vampunicorn" then
+          local str = input:read("*line")
+          if str==nil then break end
+          local race, lang, email = str:match("([^ ]*) ([^ ]*) ([^ ]*)")
+          f.race = race:lower()
+          f.options = f.options + 4096
+          f.email = email
+          f.locale = lang
+          for u in f.units do
+            u.race = race:lower()
+            u.hp = u.hp_max
+            local b = building.create(u.region, "castle")
+            if lang=="de" then
+              u.name = "Entdecker"
+              b.name = "Heimat"
+            else
+              u.name = "Explorer"
+              b.name = "Home"
+            end
+            b.size = 10
+            u.building = b
+          end
         end
-        b.size = 10
-        u.building = b
       end
-    end
+      input:close()
   end
   for r in regions() do
     r:set_resource("sapling", r:get_resource("tree")/4)

scripts/tools/build-e4.lua

@@ -27,27 +27,29 @@ else
 end
 
 local f=assert(io.open("factions", "r"))
-line=f:read("*line")
-players = {}
-emails = {}
-patrons = {}
-nplayers = 0
-while line~=nil do
-    fields = {}
-    line:gsub("([^\t]*)\t*", function(c) table.insert(fields, c) end)
+if f then
     line=f:read("*line")
-    email = fields[1]
-    if fields[2]=='yes' then
-        table.insert(patrons, email)
-    else
-        table.insert(emails, email)
+    players = {}
+    emails = {}
+    patrons = {}
+    nplayers = 0
+    while line~=nil do
+        fields = {}
+        line:gsub("([^\t]*)\t*", function(c) table.insert(fields, c) end)
+        line=f:read("*line")
+        email = fields[1]
+        if fields[2]=='yes' then
+            table.insert(patrons, email)
+        else
+            table.insert(emails, email)
+        end
+        if fields[3]=='German' then lang='de' else lang='en' end
+        race=string.gsub(fields[4], "/.*", ''):lower()
+        players[email] = { ['lang'] = lang, ['race'] = race }
+        nplayers = nplayers + 1
     end
-    if fields[3]=='German' then lang='de' else lang='en' end
-    race=string.gsub(fields[4], "/.*", ''):lower()
-    players[email] = { ['lang'] = lang, ['race'] = race }
-    nplayers = nplayers + 1
+    f:close()
 end
-
 for k, r in ipairs(homes) do
     print(k, r)
 end

scripts/tools/build.lua

@@ -27,27 +27,29 @@ else
 end
 
 local f=assert(io.open("factions", "r"))
-line=f:read("*line")
-players = {}
-emails = {}
-patrons = {}
-nplayers = 0
-while line~=nil do
-    fields = {}
-    line:gsub("([^\t]*)\t*", function(c) table.insert(fields, c) end)
+if f then
     line=f:read("*line")
-    email = fields[1]
-    if fields[2]=='yes' then
-        table.insert(patrons, email)
-    else
-        table.insert(emails, email)
+    players = {}
+    emails = {}
+    patrons = {}
+    nplayers = 0
+    while line~=nil do
+        fields = {}
+        line:gsub("([^\t]*)\t*", function(c) table.insert(fields, c) end)
+        line=f:read("*line")
+        email = fields[1]
+        if fields[2]=='yes' then
+            table.insert(patrons, email)
+        else
+            table.insert(emails, email)
+        end
+        if fields[3]=='German' then lang='de' else lang='en' end
+        race=string.gsub(fields[4], "/.*", ''):lower()
+        players[email] = { ['lang'] = lang, ['race'] = race }
+        nplayers = nplayers + 1
     end
-    if fields[3]=='German' then lang='de' else lang='en' end
-    race=string.gsub(fields[4], "/.*", ''):lower()
-    players[email] = { ['lang'] = lang, ['race'] = race }
-    nplayers = nplayers + 1
+    f:close()
 end
-
 for k, r in ipairs(homes) do
     print(k, r)
 end

src/kernel/faction.c

@@ -336,40 +336,6 @@ void write_faction_reference(const faction * f, struct storage *store)
     WRITE_INT(store, f ? f->no : 0);
 }
 
-#define DMAXHASH 7919
-typedef struct dead {
-    struct dead *nexthash;
-    faction *f;
-    int no;
-} dead;
-
-static dead *deadhash[DMAXHASH];
-
-void dhash(int no, faction * f)
-{
-    dead *hash = (dead *)calloc(1, sizeof(dead));
-    dead *old = deadhash[no % DMAXHASH];
-    hash->no = no;
-    hash->f = f;
-    deadhash[no % DMAXHASH] = hash;
-    hash->nexthash = old;
-}
-
-faction *dfindhash(int no)
-{
-    dead *old;
-
-    if (no < 0)
-        return 0;
-
-    for (old = deadhash[no % DMAXHASH]; old; old = old->nexthash) {
-        if (old->no == no) {
-            return old->f;
-        }
-    }
-    return 0;
-}
-
 void free_flist(faction **fp) {
     faction * flist = *fp;
     while (flist) {
@@ -868,6 +834,7 @@ int writepasswd(void)
 }
 
 void free_factions(void) {
+#ifdef DMAXHASH
     int i;
     for (i = 0; i != DMAXHASH; ++i) {
         while (deadhash[i]) {
@@ -876,6 +843,7 @@ void free_factions(void) {
             free(d);
         }
     }
+#endif
     free_flist(&factions);
     free_flist(&dead_factions);
 }

src/kernel/faction.h

@@ -180,10 +180,6 @@ extern "C" {
 
     struct faction *getfaction(void);
 
-    /* looking up dead factions: */
-    void dhash(int no, struct faction * f);
-    struct faction *dfindhash(int no);
-
 #ifdef __cplusplus
 }
 #endif

src/kernel/faction.test.c

@@ -191,30 +191,10 @@ static void test_set_origin_bug(CuTest *tc) {
     test_cleanup();
 }
 
-static void test_deadhash(CuTest *tc) {
-    faction *f;
-    int no;
-
-    test_setup();
-    f = test_create_faction(0);
-    no = f->no;
-    CuAssertPtrEquals(tc, f, findfaction(no));
-    CuAssertPtrEquals(tc, NULL, dfindhash(42));
-    dhash(41, f);
-    dhash(42, f);
-    assert(f == factions);
-    destroyfaction(&factions);
-    CuAssertPtrEquals(tc, 0, findfaction(no));
-    CuAssertPtrEquals(tc, f, dfindhash(42));
-    CuAssertPtrEquals(tc, f, dfindhash(41));
-    test_cleanup();
-}
-
 CuSuite *get_faction_suite(void)
 {
     CuSuite *suite = CuSuiteNew();
     SUITE_ADD_TEST(suite, test_addfaction);
-    SUITE_ADD_TEST(suite, test_deadhash);
     SUITE_ADD_TEST(suite, test_remove_empty_factions);
     SUITE_ADD_TEST(suite, test_destroyfaction_allies);
     SUITE_ADD_TEST(suite, test_remove_empty_factions_alliance);

src/kernel/jsonconf.test.c

@@ -22,6 +22,7 @@
 #include <tests.h>
 #include <string.h>
 #include <stdio.h>
+#include <errno.h>
 
 static const struct race * race_with_flag(const char * name) {
     char data[1024];
@@ -445,7 +446,9 @@ static void test_configs(CuTest * tc)
     CuAssertPtrEquals(tc, 0, buildingtypes);
     json_config(json);
     CuAssertPtrNotNull(tc, buildingtypes);
-    unlink("test.json");
+    if (unlink("test.json")!=0 && errno==ENOENT) {
+        errno = 0;
+    }
     cJSON_Delete(json);
     test_cleanup();
 }

src/kernel/save.c

@@ -1877,7 +1877,11 @@ int writegame(const char *filename)
     join_path(datapath(), filename, path, sizeof(path));
 #ifdef HAVE_UNISTD_H
     /* make sure we don't overwrite an existing file (hard links) */
-    unlink(path);
+    if (unlink(path)!=0) {
+        if (errno==ENOENT) {
+            errno = 0;
+        }
+    }
 #endif
     F = fopen(path, "wb");
     if (!F) {

src/kernel/unit.c

@@ -366,6 +366,51 @@ int gift_items(unit * u, int flags)
 
 static unit *deleted_units = NULL;
 
+#define DMAXHASH 7919
+#undef DMAXHASH // TODO: makes dfindhash slow!
+#ifdef DMAXHASH
+typedef struct dead {
+    struct dead *nexthash;
+    faction *f;
+    int no;
+} dead;
+
+static dead *deadhash[DMAXHASH];
+
+static void dhash(int no, faction * f)
+{
+    dead *hash = (dead *)calloc(1, sizeof(dead));
+    dead *old = deadhash[no % DMAXHASH];
+    hash->no = no;
+    hash->f = f;
+    deadhash[no % DMAXHASH] = hash;
+    hash->nexthash = old;
+}
+
+faction *dfindhash(int no)
+{
+    dead *old;
+
+    if (no < 0)
+        return 0;
+
+    for (old = deadhash[no % DMAXHASH]; old; old = old->nexthash) {
+        if (old->no == no) {
+            return old->f;
+        }
+    }
+    return 0;
+}
+#else
+struct faction *dfindhash(int no) {
+    unit *u = deleted_units;
+    while (u && u->no != no) {
+        u = u->next;
+    }
+    return u ? u->faction : NULL;
+}
+#endif
+
 int remove_unit(unit ** ulist, unit * u)
 {
     int result;
@@ -379,8 +424,9 @@ int remove_unit(unit ** ulist, unit * u)
         return -1;
     }
 
-    if (u->number)
+    if (u->number) {
         set_number(u, 0);
+    }
     leave(u, true);
     u->region = NULL;
 
@@ -393,11 +439,22 @@ int remove_unit(unit ** ulist, unit * u)
         *ulist = u->next;
     }
 
+    if (u->prevF) {
+        u->prevF->nextF = u->nextF;
+    }
+    if (u->nextF) {
+        u->nextF->prevF = u->prevF;
+    }
+    u->nextF = 0;
+    u->prevF = 0;
+
     u->next = deleted_units;
     deleted_units = u;
+#ifdef DMAXHASH
     dhash(u->no, u->faction);
+#endif
+    // u_setfaction(u, NULL);
 
-    u_setfaction(u, NULL);
     u->region = NULL;
 
     return 0;

src/kernel/unit.h

@@ -174,6 +174,9 @@ extern "C" {
         const struct region *r, bool noitem);
     int remove_unit(struct unit **ulist, struct unit *u);
 
+    /* looking up dead units' factions: */
+    struct faction *dfindhash(int no);
+
 #define GIFT_SELF     1<<0
 #define GIFT_FRIENDS  1<<1
 #define GIFT_PEASANTS 1<<2

src/kernel/unit.test.c

@@ -50,14 +50,16 @@ static void test_remove_empty_units_in_region(CuTest *tc) {
     test_create_world();
 
     u = test_create_unit(test_create_faction(test_create_race("human")), findregion(0, 0));
+    u = test_create_unit(u->faction, u->region);
+    CuAssertPtrNotNull(tc, u->nextF);
     uid = u->no;
     remove_empty_units_in_region(u->region);
     CuAssertPtrNotNull(tc, findunit(uid));
     u->number = 0;
     remove_empty_units_in_region(u->region);
     CuAssertPtrEquals(tc, 0, findunit(uid));
+    CuAssertPtrEquals(tc, 0, u->nextF);
     CuAssertPtrEquals(tc, 0, u->region);
-    CuAssertPtrEquals(tc, 0, u->faction);
     test_cleanup();
 }
 
@@ -401,6 +403,34 @@ static void test_unit_description(CuTest *tc) {
     test_cleanup();
 }
 
+static void test_remove_unit(CuTest *tc) {
+    region *r;
+    unit *u;
+    faction *f;
+    int uno;
+    const resource_type *rtype;
+
+    test_setup();
+    init_resources();
+    rtype = get_resourcetype(R_SILVER);
+    r = test_create_region(0, 0, 0);
+    f = test_create_faction(0);
+    u = test_create_unit(f, r);
+    uno = u->no;
+    region_setresource(r, rtype, 0);
+    i_change(&u->items, rtype->itype, 100);
+    remove_unit(&r->units, u);
+    CuAssertIntEquals(tc, 100, region_getresource(r, rtype));
+    CuAssertIntEquals(tc, 0, u->number);
+    CuAssertPtrEquals(tc, 0, u->region);
+    CuAssertPtrEquals(tc, 0, u->items);
+    CuAssertPtrEquals(tc, 0, u->nextF);
+    CuAssertPtrEquals(tc, 0, r->units);
+    CuAssertPtrEquals(tc, 0, findunit(uno));
+    CuAssertPtrEquals(tc, f, dfindhash(uno));
+    test_cleanup();
+}
+
 CuSuite *get_unit_suite(void)
 {
     CuSuite *suite = CuSuiteNew();
@@ -409,6 +439,7 @@ CuSuite *get_unit_suite(void)
     SUITE_ADD_TEST(suite, test_unit_name);
     SUITE_ADD_TEST(suite, test_unit_name_from_race);
     SUITE_ADD_TEST(suite, test_update_monster_name);
+    SUITE_ADD_TEST(suite, test_remove_unit);
     SUITE_ADD_TEST(suite, test_remove_empty_units);
     SUITE_ADD_TEST(suite, test_remove_units_ignores_spells);
     SUITE_ADD_TEST(suite, test_remove_units_without_faction);

src/names.c

@@ -232,11 +232,11 @@ static void dragon_name(unit * u)
 {
     char name[NAMESIZE + 1];
     int rnd, ter = 0;
-    int anzahl = 1;
     static int num_postfix;
     char zText[32];
     const char *str;
 
+    assert(u);
     if (num_postfix == 0) {
         for (num_postfix = 0;; ++num_postfix) {
             sprintf(zText, "dragon_postfix_%d", num_postfix);
@@ -248,26 +248,22 @@ static void dragon_name(unit * u)
             num_postfix = -1;
     }
 
-    if (u) {
-        region *r = u->region;
-        anzahl = u->number;
-        switch (rterrain(r)) {
-        case T_PLAIN:
-            ter = 1;
-            break;
-        case T_MOUNTAIN:
-            ter = 2;
-            break;
-        case T_DESERT:
-            ter = 3;
-            break;
-        case T_SWAMP:
-            ter = 4;
-            break;
-        case T_GLACIER:
-            ter = 5;
-            break;
-        }
+    switch (rterrain(u->region)) {
+    case T_PLAIN:
+        ter = 1;
+        break;
+    case T_MOUNTAIN:
+        ter = 2;
+        break;
+    case T_DESERT:
+        ter = 3;
+        break;
+    case T_SWAMP:
+        ter = 4;
+        break;
+    case T_GLACIER:
+        ter = 5;
+        break;
     }
 
     if (num_postfix <=0) {
@@ -285,7 +281,7 @@ static void dragon_name(unit * u)
     str = locale_getstring(default_locale, zText);
     assert(str != NULL);
 
-    if (anzahl > 1) {
+    if (u->number > 1) {
         const char *no_article = strchr((const char *)str, ' ');
         assert(no_article);
         // TODO: localization
@@ -308,7 +304,7 @@ static void dragon_name(unit * u)
             sz += strlcat(name, " ", sizeof(name));
             sz += strlcat(name, n, sizeof(name));
         }
-        if (u && (rng_int() % 3 == 0)) {
+        if (rng_int() % 3 == 0) {
             sz += strlcat(name, " von ", sizeof(name));
             sz += strlcat(name, (const char *)rname(u->region, default_locale), sizeof(name));
         }