Merge pull request #369 from ennorehling/coverity-leaks

make it easier for coverity to understand this code

critbit

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

src/kernel/save.c

@@ -1743,10 +1743,8 @@ int writegame(const char *filename)
 
     sprintf(path, "%s/%s", datapath(), filename);
 #ifdef HAVE_UNISTD_H
-    if (access(path, R_OK) == 0) {
-        /* make sure we don't overwrite some hardlinkedfile */
-        unlink(path);
-    }
+    /* make sure we don't overwrite an existing file (hard links) */
+    unlink(path);
 #endif
     F = fopen(path, "wb");
     if (!F) {

src/kernel/save.test.c

@@ -38,6 +38,7 @@ static void test_readwrite_unit(CuTest * tc)
     f = test_create_faction(0);
     fno = f->no;
     u = test_create_unit(f, r);
+    _mkdir(datapath());
     sprintf(path, "%s/%s", datapath(), filename);
 
     data = gamedata_open(path, "wb");

src/kernel/spell.c

@@ -73,7 +73,7 @@ spell * create_spell(const char * name, unsigned int id)
     }
     sp = (spell *)calloc(1, sizeof(spell));
     len = cb_new_kv(name, len, &sp, sizeof(sp), buffer);
-    if (cb_insert(&cb_spells, buffer, len)) {
+    if (cb_insert(&cb_spells, buffer, len) == CB_SUCCESS) {
         sp->id = id ? id : hashstring(name);
         sp->sname = _strdup(name);
         add_spell(&spells, sp);

src/magic.c

@@ -2981,7 +2981,10 @@ spellbook * get_spellbook(const char * name)
         result = create_spellbook(name);
         assert(strlen(name) + sizeof(result) < sizeof(buffer));
         len = cb_new_kv(name, len, &result, sizeof(result), buffer);
-        cb_insert(&cb_spellbooks, buffer, len);
+        if (cb_insert(&cb_spellbooks, buffer, len) == CB_EXISTS) {
+            log_error("cb_insert failed although cb_find returned nothing for spellbook=%s", name);
+            assert(!"should not happen");
+        }
     }
     return result;
 }

src/reports.c

@@ -1540,6 +1540,15 @@ static void prepare_report(struct report_context *ctx, faction *f)
     ctx->last = lastregion(f);
 }
 
+static void mkreportdir(const char *rpath) {
+    if (_access(rpath, 0) < 0) {
+        if (_mkdir(rpath) != 0) {
+            log_error("could not create reports directory %s: %s", rpath, strerror(errno));
+            abort();
+        }
+    }
+}
+
 int write_reports(faction * f, time_t ltime)
 {
     unsigned int backup = 1, maxbackup = 128 * 1000;
@@ -1547,19 +1556,14 @@ int write_reports(faction * f, time_t ltime)
     struct report_context ctx;
     const char *encoding = "UTF-8";
     report_type *rtype;
-    const char *path = reportpath();;
+    const char *path = reportpath();
 
     if (noreports) {
         return false;
     }
     prepare_report(&ctx, f);
     get_addresses(&ctx);
-    if (_access(path, 0) < 0) {
-        if (_mkdir(path) != 0) {
-            log_error("could not create reports directory %s: %s", path, strerror(errno));
-            abort();
-        }
-    }
+    mkreportdir(path);
     if (errno) {
         log_warning("errno was %d before writing reports", errno);
         errno = 0;
@@ -1641,12 +1645,7 @@ int init_reports(void)
             return 0;
         }
     }
-    if (_mkdir(reportpath()) != 0) {
-        if (errno != EEXIST) {
-            perror("could not create reportpath");
-            return -1;
-        }
-    }
+    mkreportdir(reportpath());
     return 0;
 }
 
@@ -1663,13 +1662,8 @@ int reports(void)
     report_donations();
     remove_empty_units();
 
-    if (_access(rpath, 0) < 0) {
-        if (_mkdir(rpath) != 0) {
-            log_error("could not create reports directory %s: %s", rpath, strerror(errno));
-            abort();
-        }
-    }
-    sprintf(path, "%s/reports.txt", reportpath());
+    mkreportdir(rpath);
+    sprintf(path, "%s/reports.txt", rpath);
     mailit = fopen(path, "w");
     if (mailit == NULL) {
         log_error("%s could not be opened!\n", path);

src/util/log.c

@@ -77,24 +77,26 @@ cp_convert(const char *format, char *buffer, size_t length, int codepage)
 
 void log_rotate(const char *filename, int maxindex)
 {
-    if (_access(filename, 4) == 0) {
-        char buffer[2][MAX_PATH];
-        int dst = 1;
-        assert(strlen(filename) < sizeof(buffer[0]) - 4);
+    char buffer[2][MAX_PATH];
+    int dst = 1;
+    assert(strlen(filename) < sizeof(buffer[0]) - 4);
 
-        sprintf(buffer[dst], "%s.%d", filename, maxindex);
-        while (maxindex > 0) {
-            int err, src = 1 - dst;
-            sprintf(buffer[src], "%s.%d", filename, --maxindex);
-            err = rename(buffer[src], buffer[dst]);
-            if (err != 0) {
-                log_error("log rotate %s: %s", buffer[dst], strerror(errno));
-            }
-            dst = src;
-        }
-        if (rename(filename, buffer[dst]) != 0) {
-            log_error("log rotate %s: %s", buffer[dst], strerror(errno));
+    sprintf(buffer[dst], "%s.%d", filename, maxindex);
+#ifdef HAVE_UNISTD_H
+    /* make sure we don't overwrite an existing file (hard links) */
+    unlink(buffer[dst]);
+#endif
+    while (maxindex > 0) {
+        int err, src = 1 - dst;
+        sprintf(buffer[src], "%s.%d", filename, --maxindex);
+        err = rename(buffer[src], buffer[dst]);
+        if (err != 0) {
+            log_debug("log rotate %s: %s", buffer[dst], strerror(errno));
         }
+        dst = src;
+    }
+    if (rename(filename, buffer[dst]) != 0) {
+        log_debug("log rotate %s: %s", buffer[dst], strerror(errno));
     }
 }