Merge pull request #364 from ennorehling/coverity-fixes

several smaller coverity fixes

critbit

@@ -1 +1 @@
-Subproject commit e538739b38593b90312831a5e52d2e3bd731069b
+Subproject commit 77130e660e2227ae740e06f38e85cd18ff728599

scripts/tests/storage.lua

@@ -25,6 +25,9 @@ function test_store_unit()
     store = storage.create(filename, "rb")
     assert_not_nil(store)
     u = store:read_unit()
+    assert_not_nil(u)
+    assert_equal(f, u.faction)
+    assert_equal(nil, u.region)
     store:close()
     os.remove(filename)
     assert_not_nil(u)

src/battle.c

@@ -300,9 +300,8 @@ fighter *select_corpse(battle * b, fighter * af)
  *
  * Untote werden nicht ausgewählt (casualties, not dead) */
 {
-    int si, di, maxcasualties = 0;
+    int si, maxcasualties = 0;
     fighter *df;
-    side *s;
 
     for (si = 0; si != b->nsides; ++si) {
         side *s = b->sides + si;
@@ -310,7 +309,9 @@ fighter *select_corpse(battle * b, fighter * af)
             maxcasualties += s->casualties;
         }
     }
-    di = (int)(rng_int() % maxcasualties);
+    if (maxcasualties > 0) {
+        int di = (int)(rng_int() % maxcasualties);
+        side *s;
         for (s = b->sides; s != b->sides + b->nsides; ++s) {
             for (df = s->fighters; df; df = df->next) {
                 /* Geflohene haben auch 0 hp, dürfen hier aber nicht ausgewählt
@@ -327,7 +328,7 @@ fighter *select_corpse(battle * b, fighter * af)
                 di -= dead;
             }
         }
-
+    }
     return NULL;
 }
 

src/bind_storage.c

@@ -33,25 +33,10 @@ static int tolua_storage_create(lua_State * L)
 {
     const char *filename = tolua_tostring(L, 1, 0);
     const char *type = tolua_tostring(L, 2, "rb");
-    FILE * F;
+    gamedata *data;
 
-    F = fopen(filename, type);
+    data = gamedata_open(filename, type);
-    if (F) {
+    if (data) {
-        gamedata *data = (gamedata *)calloc(1, sizeof(gamedata));
-        storage *store = (storage *)calloc(1, sizeof(storage));
-        data->store = store;
-        if (strchr(type, 'r')) {
-            fread(&data->version, sizeof(int), 1, F);
-            fseek(F, sizeof(int), SEEK_CUR);
-        }
-        else if (strchr(type, 'w')) {
-            int n = STREAM_VERSION;
-            data->version = RELEASE_VERSION;
-            fwrite(&data->version, sizeof(int), 1, F);
-            fwrite(&n, sizeof(int), 1, F);
-        }
-        fstream_init(&data->strm, F);
-        binstore_init(store, &data->strm);
         tolua_pushusertype(L, (void *)data, TOLUA_CAST "storage");
         return 1;
     }
@@ -121,8 +106,7 @@ static int tolua_storage_tostring(lua_State * L)
 static int tolua_storage_close(lua_State * L)
 {
     gamedata *data = (gamedata *)tolua_tousertype(L, 1, 0);
-    binstore_done(data->store);
+    gamedata_close(data);
-    fstream_done(&data->strm);
     return 0;
 }
 

src/bindings.c

@@ -250,11 +250,15 @@ static int tolua_message_faction(lua_State * L)
     unit *sender = (unit *)tolua_tousertype(L, 1, 0);
     faction *target = (faction *)tolua_tousertype(L, 2, 0);
     const char *str = tolua_tostring(L, 3, 0);
-    if (!target)
+    if (!target) {
         tolua_error(L, TOLUA_CAST "target is nil", NULL);
-    if (!sender)
+    }
+    else if (!sender) {
         tolua_error(L, TOLUA_CAST "sender is nil", NULL);
+    }
+    else {
         deliverMail(target, sender->region, sender, str, NULL);
+    }
     return 0;
 }
 

src/kernel/faction.c

@@ -160,6 +160,9 @@ const unit *random_unit_in_faction(const faction * f)
     unit *u;
     int c = 0, u_nr;
 
+    if (!f->units) {
+        return NULL;
+    }
     for (u = f->units; u; u = u->next)
         c++;
 
@@ -593,10 +596,12 @@ int skill_limit(faction * f, skill_t sk)
         if (sk != SK_ALCHEMY && sk != SK_MAGIC)
             return INT_MAX;
         if (f_get_alliance(f)) {
-            int ac = listlen(f->alliance->members);   /* number of factions */
+            int sc, fl, ac = listlen(f->alliance->members);   /* number of factions */
-            int fl = (al + ac - 1) / ac;      /* faction limit, rounded up */
+
+            assert(ac > 0);
+            fl = (al + ac - 1) / ac;      /* faction limit, rounded up */
             /* the faction limit may not be achievable because it would break the alliance-limit */
-            int sc = al - allied_skillcount(f, sk);
+            sc = al - allied_skillcount(f, sk);
             if (sc <= 0)
                 return 0;
             return fl;

src/kernel/jsonconf.c

@@ -839,8 +839,9 @@ static void json_include(cJSON *json) {
             fseek(F, 0, SEEK_END);
             sz = ftell(F);
             rewind(F);
-            data = malloc(sz);
+            data = malloc(sz+1);
-            fread(data, 1, sz, F);
+            sz = fread(data, 1, sz, F);
+            data[sz] = 0;
             fclose(F);
             config = cJSON_Parse(data);
             free(data);

src/kernel/race.c

@@ -244,7 +244,7 @@ const char *raceprefix(const unit * u)
     const attrib *asource = u->faction->attribs;
 
     if (fval(u, UFL_GROUP)) {
-        const attrib *agroup = agroup = a_findc(u->attribs, &at_group);
+        const attrib *agroup = a_findc(u->attribs, &at_group);
         if (agroup != NULL)
             asource = ((const group *)(agroup->data.v))->attribs;
     }

src/kernel/save.c

@@ -613,8 +613,7 @@ unit *read_unit(struct gamedata *data)
         ++u->faction->no_units;
     }
     else {
-        log_error("unit %s has faction == NULL\n", unitname(u));
+        log_error("unit %s has faction == NULL\n", itoa36(u->no));
-        assert(u->faction);
         return 0;
     }
 
@@ -1428,9 +1427,14 @@ int readgame(const char *filename, bool backup)
 
         READ_INT(&store, &gameid);
         if (gameid != game_id()) {
-            log_warning("game mismatch: datafile contains game %d, but config is for %d\n", gameid, game_id());
+            int c;
+            log_warning("game mismatch: datafile contains game %d, but config is for %d", gameid, game_id());
             printf("WARNING: invalid game id. any key to continue, Ctrl-C to stop\n");
-            getchar();
+            c = getchar();
+            if (c == EOF) {
+                log_error("aborting.");
+                abort();
+            }
         }
     }
     else {
@@ -1883,6 +1887,50 @@ int writegame(const char *filename)
     return 0;
 }
 
+void gamedata_close(gamedata *data) {
+    binstore_done(data->store);
+    fstream_done(&data->strm);
+}
+
+gamedata *gamedata_open(const char *filename, const char *mode) {
+    FILE *F = fopen(filename, mode);
+
+    if (F) {
+        gamedata *data = (gamedata *)calloc(1, sizeof(gamedata));
+        storage *store = (storage *)calloc(1, sizeof(storage));
+        int err = 0;
+        size_t sz;
+
+        data->store = store;
+        if (strchr(mode, 'r')) {
+            sz = fread(&data->version, 1, sizeof(int), F);
+            if (sz != sizeof(int)) {
+                err = ferror(F);
+            }
+            else {
+                err = fseek(F, sizeof(int), SEEK_CUR);
+            }
+        }
+        else if (strchr(mode, 'w')) {
+            int n = STREAM_VERSION;
+            data->version = RELEASE_VERSION;
+            fwrite(&data->version, sizeof(int), 1, F);
+            fwrite(&n, sizeof(int), 1, F);
+        }
+        if (err) {
+            fclose(F);
+            free(data);
+            free(store);
+        }
+        else {
+            fstream_init(&data->strm, F);
+            binstore_init(store, &data->strm);
+            return data;
+        }
+    }
+    return 0;
+}
+
 int a_readint(attrib * a, void *owner, struct storage *store)
 {
     /*  assert(sizeof(int)==sizeof(a->data)); */

src/kernel/save.h

@@ -42,44 +42,46 @@ extern "C" {
     /* Nach MAX_INPUT_SIZE brechen wir das Einlesen der Zeile ab und nehmen an,
      * dass hier ein Fehler (fehlende ") vorliegt */
 
+    extern int data_version;
+    extern int enc_gamedata;
+
     int readorders(const char *filename);
     int creategame(void);
     int readgame(const char *filename, bool backup);
     int writegame(const char *filename);
 
-    /* Versionsänderungen: */
+    int current_turn(void);
-    extern int data_version;
-    extern int enc_gamedata;
 
-    extern int current_turn(void);
+    void read_items(struct storage *store, struct item **it);
+    void write_items(struct storage *store, struct item *it);
 
-    extern void read_items(struct storage *store, struct item **it);
+    void read_spellbook(struct spellbook **bookp, struct storage *store, int(*get_level)(const struct spell * sp, void *), void * cbdata);
-    extern void write_items(struct storage *store, struct item *it);
+    void write_spellbook(const struct spellbook *book, struct storage *store);
 
-    extern void read_spellbook(struct spellbook **bookp, struct storage *store, int(*get_level)(const struct spell * sp, void *), void * cbdata);
+    void write_unit(struct gamedata *data, const struct unit *u);
-    extern void write_spellbook(const struct spellbook *book, struct storage *store);
+    struct unit *read_unit(struct gamedata *data);
 
-    extern void write_unit(struct gamedata *data, const struct unit *u);
+    int a_readint(struct attrib *a, void *owner, struct storage *store);
-    extern struct unit *read_unit(struct gamedata *data);
+    void a_writeint(const struct attrib *a, const void *owner,
-
-    extern int a_readint(struct attrib *a, void *owner, struct storage *store);
-    extern void a_writeint(const struct attrib *a, const void *owner,
         struct storage *store);
-    extern int a_readshorts(struct attrib *a, void *owner, struct storage *store);
+    int a_readshorts(struct attrib *a, void *owner, struct storage *store);
-    extern void a_writeshorts(const struct attrib *a, const void *owner,
+    void a_writeshorts(const struct attrib *a, const void *owner,
         struct storage *store);
-    extern int a_readchars(struct attrib *a, void *owner, struct storage *store);
+    int a_readchars(struct attrib *a, void *owner, struct storage *store);
-    extern void a_writechars(const struct attrib *a, const void *owner,
+    void a_writechars(const struct attrib *a, const void *owner,
         struct storage *store);
-    extern int a_readvoid(struct attrib *a, void *owner, struct storage *store);
+    int a_readvoid(struct attrib *a, void *owner, struct storage *store);
-    extern void a_writevoid(const struct attrib *a, const void *owner,
+    void a_writevoid(const struct attrib *a, const void *owner,
         struct storage *store);
-    extern int a_readstring(struct attrib *a, void *owner, struct storage *store);
+    int a_readstring(struct attrib *a, void *owner, struct storage *store);
-    extern void a_writestring(const struct attrib *a, const void *owner,
+    void a_writestring(const struct attrib *a, const void *owner,
         struct storage *store);
-    extern void a_finalizestring(struct attrib *a);
+    void a_finalizestring(struct attrib *a);
 
-    extern void create_backup(char *file);
+    void create_backup(char *file);
+
+    struct gamedata *gamedata_open(const char *filename, const char *mode);
+    void gamedata_close(struct gamedata *data);
 
 #ifdef __cplusplus
 }

src/kernel/save.test.c

@@ -2,6 +2,8 @@
 #include <kernel/config.h>
 
 #include "save.h"
+#include "unit.h"
+#include "faction.h"
 #include "version.h"
 #include <CuTest.h>
 #include <tests.h>
@@ -21,9 +23,45 @@ static void test_readwrite_data(CuTest * tc)
     test_cleanup();
 }
 
+static void test_readwrite_unit(CuTest * tc)
+{
+    const char *filename = "test.dat";
+    char path[MAX_PATH];
+    gamedata *data;
+    struct unit *u;
+    struct region *r;
+    struct faction *f;
+    int fno;
+
+    test_cleanup();
+    r = test_create_region(0, 0, 0);
+    f = test_create_faction(0);
+    fno = f->no;
+    u = test_create_unit(f, r);
+    sprintf(path, "%s/%s", datapath(), filename);
+
+    data = gamedata_open(path, "wb");
+    write_unit(data, u);
+    gamedata_close(data);
+
+    free_gamedata();
+    f = test_create_faction(0);
+    renumber_faction(f, fno);
+    data = gamedata_open(path, "rb");
+    u = read_unit(data);
+    gamedata_close(data);
+
+    CuAssertPtrNotNull(tc, u);
+    CuAssertPtrEquals(tc, f, u->faction);
+    CuAssertPtrEquals(tc, 0, u->region);
+    CuAssertIntEquals(tc, 0, remove(path));
+    test_cleanup();
+}
+
 CuSuite *get_save_suite(void)
 {
     CuSuite *suite = CuSuiteNew();
     SUITE_ADD_TEST(suite, test_readwrite_data);
+    SUITE_ADD_TEST(suite, test_readwrite_unit);
     return suite;
 }

src/modules/autoseed.c

@@ -54,6 +54,7 @@ const terrain_type *random_terrain(const terrain_type * terrains[],
     const terrain_type *terrain;
     int n;
 
+    assert(size > 0);
     if (distribution) {
         ndistribution = 0;
         for (n = 0; n != size; ++n) {

src/names.c

@@ -91,7 +91,7 @@ static const char *make_names(const char *monster, int *num_postfix,
     uu = rng_int() % *num_name;
 
     /* nur 50% aller Namen haben "Nach-Teil", wenn kein Vor-Teil */
-    if (uv >= *num_prefix) {
+    if (*num_postfix > 0 && uv >= *num_prefix) {
         un = rng_int() % *num_postfix;
     }
     else {
@@ -421,8 +421,7 @@ const char *abkz(const char *s, char *buf, size_t buflen, size_t maxchars)
     }
 
     /* Buchstaben pro Teilkürzel = _max(1,max/AnzWort) */
-
+    bpt = (c > 0) ? _max(1, maxchars / c) : 1;
-    bpt = _max(1, maxchars / c);
 
     /* Einzelne Wörter anspringen und jeweils die ersten BpT kopieren */
 

src/spells/flyingship.c

@@ -154,10 +154,12 @@ static curse *shipcurse_flyingship(ship * sh, unit * mage, double power, int dur
         /* mit C_SHIP_NODRIFT haben wir kein Problem */
         curse *c =
             create_curse(mage, &sh->attribs, ct_flyingship, power, duration, 0.0, 0);
+        if (c) {
             c->data.v = sh;
-        if (c && c->duration > 0) {
+            if (c->duration > 0) {
                 sh->flags |= SF_FLYING;
             }
+        }
         return c;
     }
 }

src/util/umlaut.c

@@ -97,7 +97,12 @@ char * transliterate(char * out, size_t size, const char * in)
             }
             else {
                 ucs4_t ucs;
-                unicode_utf8_to_ucs4(&ucs, src, &len);
+                int ret = unicode_utf8_to_ucs4(&ucs, src, &len);
+                if (ret != 0) {
+                    /* encoding is broken. yikes */
+                    log_error("transliterate | encoding error in '%s'\n", src);
+                    return NULL;
+                }
                 src += len;
                 *dst++ = '?';
                 --size;

storage

@@ -1 +1 @@
-Subproject commit 86b96744157eb08c55998df4c12fa2e073005b49
+Subproject commit 1d92cb36df41c183c378aad17cbbfc0eddbb5c84