Changeset: 6982cdf2d64b for MonetDB
URL: https://dev.monetdb.org/hg/MonetDB?cmd=changeset;node=6982cdf2d64b
Modified Files:
gdk/gdk_bat.c
gdk/gdk_batop.c
gdk/gdk_bbp.c
gdk/gdk_firstn.c
gdk/gdk_heap.c
gdk/gdk_imprints.c
gdk/gdk_join.c
gdk/gdk_logger.c
gdk/gdk_select.c
gdk/gdk_storage.c
gdk/gdk_string.c
gdk/gdk_utils.c
Branch: Nov2019
Log Message:
Coverity fixes.
diffs (truncated from 359 to 300 lines):
diff --git a/gdk/gdk_bat.c b/gdk/gdk_bat.c
--- a/gdk/gdk_bat.c
+++ b/gdk/gdk_bat.c
@@ -1840,6 +1840,10 @@ backup_new(Heap *hp, int lockbat)
/* check for an existing X.new in BATDIR, BAKDIR and SUBDIR */
batpath = GDKfilepath(hp->farmid, BATDIR, hp->filename, ".new");
bakpath = GDKfilepath(hp->farmid, BAKDIR, hp->filename, ".new");
+ if (batpath == NULL || bakpath == NULL) {
+ ret = -1;
+ goto bailout;
+ }
batret = stat(batpath, &st);
bakret = stat(bakpath, &st);
@@ -1856,6 +1860,7 @@ backup_new(Heap *hp, int lockbat)
GDKsyserror("backup_new: remove %s failed\n", batpath);
IODEBUG fprintf(stderr, "#remove(%s) = %d\n", batpath, ret);
}
+ bailout:
GDKfree(batpath);
GDKfree(bakpath);
for (xx = lockbat; xx >= 0; xx--)
diff --git a/gdk/gdk_batop.c b/gdk/gdk_batop.c
--- a/gdk/gdk_batop.c
+++ b/gdk/gdk_batop.c
@@ -49,6 +49,10 @@ unshare_string_heap(BAT *b)
* it makes sense to just quickly copy the whole string heap instead
* of inserting individual strings. See the comments in the code for
* more information. */
+#ifdef STATIC_CODE_ANALYSIS
+#define rand() 0
+#endif
+
static gdk_return
insert_string_bat(BAT *b, BAT *n, BAT *s, bool force)
{
diff --git a/gdk/gdk_bbp.c b/gdk/gdk_bbp.c
--- a/gdk/gdk_bbp.c
+++ b/gdk/gdk_bbp.c
@@ -403,6 +403,7 @@ fixfltheap(BAT *b)
if (srcdir == NULL) {
return GDK_FAIL;
}
+ /* coverity[null_returns] */
*strrchr(srcdir, DIR_SEP) = 0;
if ((bnme = strrchr(nme, DIR_SEP)) != NULL)
@@ -690,6 +691,7 @@ fixdateheap(BAT *b, const char *anme)
if (srcdir == NULL) {
return GDK_FAIL;
}
+ /* coverity[null_returns] */
*strrchr(srcdir, DIR_SEP) = 0;
if ((bnme = strrchr(nme, DIR_SEP)) != NULL)
@@ -1045,7 +1047,7 @@ BBPreadEntries(FILE *fp, unsigned bbpver
int nread, n;
char *s, *options = NULL;
char logical[1024];
- uint64_t first = 0, count, capacity, base = 0;
+ uint64_t count, capacity, base = 0;
int Thashash;
static_assert(sizeof(BBP_physical(0)) == sizeof(filename),
@@ -1089,12 +1091,6 @@ BBPreadEntries(FILE *fp, unsigned bbpver
*s++ = DIR_SEP;
#endif
- if (first != 0) {
- GDKerror("BBPinit: first != 0 (ID = %" PRIu64 ").",
- batid);
- return GDK_FAIL;
- }
-
bid = (bat) batid;
if (batid >= (uint64_t) ATOMIC_GET(&BBPsize)) {
ATOMIC_SET(&BBPsize, batid + 1);
@@ -1476,8 +1472,11 @@ BBPinit(void)
} else if (GDKmove(0, BATDIR, "BBP", "bak", BATDIR,
"BBP", "dir") == GDK_SUCCEED)
IODEBUG fprintf(stderr, "#BBPinit: reverting to
dir saved in BBP.bak.\n");
- if ((fp = GDKfilelocate(0, "BBP", "r", "dir")) == NULL)
+ if ((fp = GDKfilelocate(0, "BBP", "r", "dir")) == NULL)
{
+ GDKfree(bbpdirstr);
+ GDKfree(backupbbpdirstr);
goto bailout;
+ }
}
assert(fp != NULL);
GDKfree(bbpdirstr);
@@ -1779,7 +1778,7 @@ BBPdir_subcommit(int cnt, bat *subcommit
obbpf = NULL;
} else if (sscanf(buf, "%d", &n) != 1 || n <= 0) {
GDKerror("BBPdir: subcommit attempted with
invalid backup BBP.dir.");
- return GDK_FAIL;
+ goto bailout;
}
/* at this point, obbpf == NULL, or n > 0 */
}
@@ -3369,10 +3368,18 @@ BBPsync(int cnt, bat *subcommit)
char *f;
snprintf(o, sizeof(o), "%o", (unsigned)
b->batCacheid);
f = GDKfilepath(b->theap.farmid, BAKDIR, o,
"tail");
+ if (f == NULL) {
+ ret = GDK_FAIL;
+ goto bailout;
+ }
if (access(f, F_OK) == 0)
file_move(b->theap.farmid, BAKDIR,
SUBDIR, o, "tail");
GDKfree(f);
f = GDKfilepath(b->theap.farmid, BAKDIR, o,
"theap");
+ if (f == NULL) {
+ ret = GDK_FAIL;
+ goto bailout;
+ }
if (access(f, F_OK) == 0)
file_move(b->theap.farmid, BAKDIR,
SUBDIR, o, "theap");
GDKfree(f);
@@ -3440,7 +3447,10 @@ BBPsync(int cnt, bat *subcommit)
backup_files = 1;
}
}
- PERFDEBUG fprintf(stderr, "#BBPsync (ready time %d)\n", (t0 = GDKms())
- t1);
+ PERFDEBUG fprintf(stderr, "#BBPsync%s (ready time %d)\n",
+ ret == GDK_SUCCEED ? "" : " failed",
+ (t0 = GDKms()) - t1);
+ bailout:
GDKfree(bakdir);
GDKfree(deldir);
return ret;
@@ -3614,8 +3624,12 @@ BBPrecover(int farmid)
char *fn;
fn = GDKfilepath(farmid, BATDIR, "BBP", "dir");
- ret = recover_dir(farmid, stat(fn, &st) == 0);
- GDKfree(fn);
+ if (fn == NULL) {
+ ret = GDK_FAIL;
+ } else {
+ ret = recover_dir(farmid, stat(fn, &st) == 0);
+ GDKfree(fn);
+ }
}
if (ret == GDK_SUCCEED) {
diff --git a/gdk/gdk_firstn.c b/gdk/gdk_firstn.c
--- a/gdk/gdk_firstn.c
+++ b/gdk/gdk_firstn.c
@@ -392,7 +392,7 @@ BATfirstn_unique(BAT *b, BAT *s, BUN n,
/* prefer nil and largest */
if (pos < n) {
if (lastp)
- *lastp = cand[cnt - (n
- pos)];
+ *lastp = b->hseqbase +
end - (n - pos);
return doublerange(b->hseqbase
+ start, b->hseqbase + pos, b->hseqbase + end - (n - pos), b->hseqbase + end);
}
if (lastp)
diff --git a/gdk/gdk_heap.c b/gdk/gdk_heap.c
--- a/gdk/gdk_heap.c
+++ b/gdk/gdk_heap.c
@@ -355,6 +355,8 @@ file_exists(int farmid, const char *dir,
int ret;
path = GDKfilepath(farmid, dir, name, ext);
+ if (path == NULL)
+ return -1;
ret = stat(path, &st);
IODEBUG fprintf(stderr, "#stat(%s) = %d\n", path, ret);
GDKfree(path);
@@ -410,8 +412,9 @@ GDKupgradevarheap(BAT *b, var_t v, bool
filename = b->theap.filename;
else
filename++;
+ int exists = 0;
if ((BBP_status(bid) & (BBPEXISTING|BBPDELETED)) &&
- !file_exists(b->theap.farmid, BAKDIR, filename, NULL) &&
+ !(exists = file_exists(b->theap.farmid, BAKDIR, filename, NULL)) &&
(b->theap.storage != STORE_MEM ||
GDKmove(b->theap.farmid, BATDIR, b->theap.filename, NULL,
BAKDIR, filename, NULL) != GDK_SUCCEED)) {
@@ -455,6 +458,8 @@ GDKupgradevarheap(BAT *b, var_t v, bool
return GDK_FAIL;
}
}
+ if (exists == -1)
+ return GDK_FAIL;
savefree = b->theap.free;
if (copyall)
diff --git a/gdk/gdk_imprints.c b/gdk/gdk_imprints.c
--- a/gdk/gdk_imprints.c
+++ b/gdk/gdk_imprints.c
@@ -395,7 +395,6 @@ BATimprints(BAT *b)
imprints = GDKzalloc(sizeof(Imprints));
if (imprints == NULL) {
- MT_lock_unset(&b->batIdxLock);
return GDK_FAIL;
}
stpconcat(imprints->imprints.filename, nme, ".timprints", NULL);
@@ -406,20 +405,17 @@ BATimprints(BAT *b)
#define SMP_SIZE 2048
s1 = BATsample(b, SMP_SIZE);
if (s1 == NULL) {
- MT_lock_unset(&b->batIdxLock);
GDKfree(imprints);
return GDK_FAIL;
}
s2 = BATunique(b, s1);
if (s2 == NULL) {
- MT_lock_unset(&b->batIdxLock);
BBPunfix(s1->batCacheid);
GDKfree(imprints);
return GDK_FAIL;
}
s3 = BATproject(s2, b);
if (s3 == NULL) {
- MT_lock_unset(&b->batIdxLock);
BBPunfix(s1->batCacheid);
BBPunfix(s2->batCacheid);
GDKfree(imprints);
@@ -427,7 +423,6 @@ BATimprints(BAT *b)
}
s3->tkey = true; /* we know is unique on tail now */
if (BATsort(&s4, NULL, NULL, s3, NULL, NULL, false, false,
false) != GDK_SUCCEED) {
- MT_lock_unset(&b->batIdxLock);
BBPunfix(s1->batCacheid);
BBPunfix(s2->batCacheid);
BBPunfix(s3->batCacheid);
diff --git a/gdk/gdk_join.c b/gdk/gdk_join.c
--- a/gdk/gdk_join.c
+++ b/gdk/gdk_join.c
@@ -663,7 +663,7 @@ mergejoin_void(BAT **r1p, BAT **r2p, BAT
/* l is dense, but the candidate list exists and is
* not dense; we can, by manipulating the range
* [lo..hi), just look at the candidate list values */
- assert(!BATtdense(sl));
+ assert(sl && !BATtdense(sl));
/* translate lo and hi to l's OID values that now need
* to match */
diff --git a/gdk/gdk_logger.c b/gdk/gdk_logger.c
--- a/gdk/gdk_logger.c
+++ b/gdk/gdk_logger.c
@@ -2688,6 +2688,7 @@ logger_cleanup(logger *lg)
len = snprintf(log_id, sizeof(log_id), LLFMT, lid);
if (len == -1 || len >= FILENAME_MAX) {
fprintf(stderr, "#logger_cleanup: log_id filename is
too large\n");
+ fclose(fp);
return GDK_FAIL;
}
if (GDKunlink(farmid, lg->dir, LOGFILE, log_id) != GDK_SUCCEED)
{
diff --git a/gdk/gdk_select.c b/gdk/gdk_select.c
--- a/gdk/gdk_select.c
+++ b/gdk/gdk_select.c
@@ -1834,7 +1834,7 @@ BATselect(BAT *b, BAT *s, const void *tl
* to do a binary search on the candidate list (or 1
* if no need for search)) */
tmp = BBPquickdesc(parent, false);
- hash = phash = BATcheckhash(tmp) &&
+ hash = phash = tmp && BATcheckhash(tmp) &&
(BATcount(tmp) == BATcount(b) ||
BATcount(tmp) / ((size_t *) tmp->thash->heap.base)[5]
* (s && !BATtdense(s) ? ilog2(BATcount(s)) : 1) < (s ? BATcount(s) :
BATcount(b)) ||
HASHget(tmp->thash, HASHprobe(tmp->thash, tl)) ==
HASHnil(tmp->thash));
diff --git a/gdk/gdk_storage.c b/gdk/gdk_storage.c
--- a/gdk/gdk_storage.c
+++ b/gdk/gdk_storage.c
@@ -170,10 +170,15 @@ GDKremovedir(int farmid, const char *dir
continue;
}
path = GDKfilepath(farmid, dirname, dent->d_name, NULL);
+ if (path == NULL) {
+ ret = -1;
+ goto bailout;
+ }
ret = remove(path);
IODEBUG fprintf(stderr, "#remove %s = %d\n", path, ret);
GDKfree(path);
}
+ bailout:
closedir(dirp);
ret = rmdir(dirnamestr);
if (ret != 0)
diff --git a/gdk/gdk_string.c b/gdk/gdk_string.c
--- a/gdk/gdk_string.c
+++ b/gdk/gdk_string.c
@@ -1045,7 +1045,7 @@ concat_strings(BAT **bnp, ValPtr pt, BAT
}
for (i = 0; i < ngrp; i++) {
if (astrings[i] == NULL) {
- if ((astrings[i] =
GDKmalloc((lengths[i] + 1 - separator_length) * sizeof(str))) == NULL) {
+ if ((astrings[i] = GDKmalloc(lengths[i]
+ 1 - separator_length)) == NULL) {
rres = GDK_FAIL;
goto finish;
}
@@ -1105,7 +1105,7 @@ concat_strings(BAT **bnp, ValPtr pt, BAT
}
for (i = 0; i < ngrp; i++) {
if (astrings[i] == NULL) {
- if ((astrings[i] =
GDKmalloc((lengths[i] + 1 - separator_length) * sizeof(str))) == NULL) {
_______________________________________________
checkin-list mailing list
[email protected]
https://www.monetdb.org/mailman/listinfo/checkin-list