Changeset: 1cff4b89ae28 for MonetDB
URL: https://dev.monetdb.org/hg/MonetDB/rev/1cff4b89ae28
Modified Files:
gdk/gdk_hash.c
gdk/gdk_logger.c
gdk/gdk_system.c
gdk/gdk_unique.c
geom/monetdb5/geod.c
geom/monetdb5/geom.c
Branch: Aug2024
Log Message:
Fixes inspired by coverity.
diffs (282 lines):
diff --git a/gdk/gdk_hash.c b/gdk/gdk_hash.c
--- a/gdk/gdk_hash.c
+++ b/gdk/gdk_hash.c
@@ -1015,6 +1015,9 @@ BAThash(BAT *b)
if (BATcheckhash(b)) {
return GDK_SUCCEED;
}
+#ifdef __COVERITY__
+ MT_rwlock_wrlock(&b->thashlock);
+#else
for (;;) {
/* If multiple threads simultaneously try to build a
* hash on a bat, e.g. in order to perform a join, it
@@ -1037,6 +1040,7 @@ BAThash(BAT *b)
MT_rwlock_rdunlock(&b->thashlock);
}
}
+#endif
/* we have the write lock */
if (b->thash == NULL) {
struct canditer ci;
diff --git a/gdk/gdk_logger.c b/gdk/gdk_logger.c
--- a/gdk/gdk_logger.c
+++ b/gdk/gdk_logger.c
@@ -1260,7 +1260,7 @@ log_read_transaction(logger *lg, uint32_
case LOG_UPDATE:
case LOG_CREATE:
case LOG_DESTROY:
- if (updated && BAThash(lg->catalog_id) == GDK_SUCCEED) {
+ if (tr != NULL && updated && BAThash(lg->catalog_id) ==
GDK_SUCCEED) {
BATiter cni = bat_iterator(lg->catalog_id);
BUN p;
BUN posnew = BUN_NONE;
@@ -3229,8 +3229,8 @@ log_tdone(logger *lg, logged_range *rang
gdk_return
log_tflush(logger *lg, ulng file_id, ulng commit_ts)
{
+ rotation_lock(lg);
if (lg->flushnow) {
- rotation_lock(lg);
logged_range *p = lg->current;
assert(lg->flush_ranges == lg->current);
assert(ATOMIC_GET(&lg->current->flushed_ts) ==
ATOMIC_GET(&lg->current->last_ts));
@@ -3247,10 +3247,11 @@ log_tflush(logger *lg, ulng file_id, uln
return log_commit(lg, p, NULL, 0);
}
- if (LOG_DISABLED(lg))
+ if (LOG_DISABLED(lg)) {
+ rotation_unlock(lg);
return GDK_SUCCEED;
-
- rotation_lock(lg);
+ }
+
logged_range *frange = do_flush_range_cleanup(lg);
while (frange->next && frange->id < file_id) {
@@ -3454,7 +3455,7 @@ log_tstart(logger *lg, bool flushnow, ul
return GDK_SUCCEED;
}
/* I am now the exclusive flusher */
- if (ATOMIC_GET(&lg->nr_flushers)) {
+ while (ATOMIC_GET(&lg->nr_flushers)) {
/* I am waiting until all existing flushers are done */
MT_cond_wait(&lg->excl_flush_cv, &lg->rotation_lock);
}
diff --git a/gdk/gdk_system.c b/gdk/gdk_system.c
--- a/gdk/gdk_system.c
+++ b/gdk/gdk_system.c
@@ -193,7 +193,7 @@ struct thread_funcs {
void *data;
};
-static struct mtthread {
+struct mtthread {
struct mtthread *next;
void (*func) (void *); /* function to be called */
void *data; /* and its data */
@@ -228,13 +228,15 @@ static struct mtthread {
uintptr_t sp;
char *errbuf;
struct freebats freebats;
-} *mtthreads = NULL;
-struct mtthread mainthread = {
+};
+static struct mtthread mainthread = {
.threadname = "main thread",
.exited = ATOMIC_VAR_INIT(0),
.refs = 1,
.tid = 1,
};
+static struct mtthread *mtthreads = &mainthread;
+
#ifdef HAVE_PTHREAD_H
static pthread_mutex_t posthread_lock = PTHREAD_MUTEX_INITIALIZER;
static pthread_key_t threadkey;
@@ -404,8 +406,6 @@ MT_thread_init(void)
}
InitializeCriticalSection(&winthread_cs);
#endif
- mainthread.next = NULL;
- mtthreads = &mainthread;
thread_initialized = true;
return true;
}
diff --git a/gdk/gdk_unique.c b/gdk/gdk_unique.c
--- a/gdk/gdk_unique.c
+++ b/gdk/gdk_unique.c
@@ -178,11 +178,8 @@ BATunique(BAT *b, BAT *s)
(b->batRole == PERSISTENT && GDKinmemory(0))) &&
ci.ncand == bi.count &&
BAThash(b) == GDK_SUCCEED)) {
- BUN lo = 0;
-
/* we already have a hash table on b, or b is
- * persistent and we could create a hash table, or b
- * is a view on a bat that already has a hash table */
+ * persistent and we could create a hash table */
algomsg = "unique: existing hash";
MT_rwlock_rdlock(&b->thashlock);
hs = b->thash;
@@ -196,18 +193,18 @@ BATunique(BAT *b, BAT *s)
o = canditer_next(&ci);
p = o - hseq;
v = VALUE(p);
- for (hb = HASHgetlink(hs, p + lo);
- hb != BUN_NONE && hb >= lo;
+ for (hb = HASHgetlink(hs, p);
+ hb != BUN_NONE;
hb = HASHgetlink(hs, hb)) {
- assert(hb < p + lo);
+ assert(hb < p);
if (cmp(v, BUNtail(bi, hb)) == 0 &&
- canditer_contains(&ci, hb - lo + hseq)) {
+ canditer_contains(&ci, hb + hseq)) {
/* we've seen this value
* before */
break;
}
}
- if (hb == BUN_NONE || hb < lo) {
+ if (hb == BUN_NONE) {
if (bunfastappTYPE(oid, bn, &o) != GDK_SUCCEED)
{
MT_rwlock_rdunlock(&b->thashlock);
hs = NULL;
diff --git a/geom/monetdb5/geod.c b/geom/monetdb5/geod.c
--- a/geom/monetdb5/geod.c
+++ b/geom/monetdb5/geod.c
@@ -349,12 +349,16 @@ static BoundingBox *
boundingBoxLines(GeoLines lines)
{
CartPoint3D c;
- BoundingBox *bb = GDKzalloc(sizeof(BoundingBox));
+ BoundingBox *bb;
//If there are no segments, return NULL
if (lines.pointCount == 0)
return NULL;
+ bb = GDKzalloc(sizeof(BoundingBox));
+ if (bb == NULL)
+ return NULL;
+
c = geo2cartFromDegrees(lines.points[0]);
//Initialize the bounding box with the first point
@@ -375,26 +379,12 @@ boundingBoxContainsPoint(BoundingBox bb,
return bb.xmin <= pt.x && bb.xmax >= pt.x && bb.ymin <= pt.y && bb.ymax
>= pt.y && bb.zmin <= pt.z && bb.zmax >= pt.z;
}
-static BoundingBox*
-boundingBoxCopy(BoundingBox bb)
-{
- //TODO Malloc fail?
- BoundingBox *copy = GDKmalloc(sizeof(BoundingBox));
- copy->xmin = bb.xmin;
- copy->xmax = bb.xmax;
- copy->ymin = bb.ymin;
- copy->ymax = bb.ymax;
- copy->zmin = bb.zmin;
- copy->zmax = bb.zmax;
- return copy;
-}
-
/* Returns a point outside of the polygon's bounding box, for Point-In-Polygon
calculation */
static GeoPoint
pointOutsidePolygon(GeoPolygon polygon)
{
- BoundingBox bb = *(polygon.bbox);
- BoundingBox *bb2 = boundingBoxCopy(*(polygon.bbox));
+ BoundingBox bb = *polygon.bbox;
+ BoundingBox bb2 = *polygon.bbox;
//TODO: From POSTGIS -> CHANGE
double grow = M_PI / 180.0 / 60.0;
@@ -446,9 +436,8 @@ pointOutsidePolygon(GeoPolygon polygon)
corners[7].z = bb.zmax;
for (int i = 0; i < 8; i++)
- if (!boundingBoxContainsPoint(*bb2, corners[i])) {
+ if (!boundingBoxContainsPoint(bb2, corners[i])) {
CartPoint3D pt_cart = corners[i];
- GDKfree(bb2);
return rad2DegPoint(cart2geo(pt_cart));
}
grow *= 2.0;
diff --git a/geom/monetdb5/geom.c b/geom/monetdb5/geom.c
--- a/geom/monetdb5/geom.c
+++ b/geom/monetdb5/geom.c
@@ -297,7 +297,7 @@ wkbCollect (wkb **out, wkb * const *a, w
int type_a, type_b;
if ((err = wkbGetCompatibleGeometries(a, b, &geoms[0], &geoms[1])) !=
MAL_SUCCEED)
- throw(MAL,"geom.Collect", "%s", err);
+ return err;
//int srid = GEOSGetSRID_r(geoshandle, ga);
type_a = GEOSGeomTypeId_r(geoshandle, geoms[0]);
@@ -3454,8 +3454,7 @@ wkbMakeLineAggrArray(wkb **outWKB, wkb *
if (size == 1) {
msg = wkbFromWKB(outWKB, &aWKB);
if (msg) {
- freeException(msg);
- throw(MAL, "aggr.MakeLine", SQLSTATE(HY013)
MAL_MALLOC_FAIL);
+ return msg;
}
return MAL_SUCCEED;
}
@@ -3464,14 +3463,21 @@ wkbMakeLineAggrArray(wkb **outWKB, wkb *
outCoordSeq = GEOSCoordSeq_create_r(geoshandle, size, 2);
msg = wkbExtractPointToCoordSeq(&outCoordSeq, aWKB, 0);
+ if (msg)
+ return msg;
msg = wkbExtractPointToCoordSeq(&outCoordSeq, bWKB, 1);
+ if (msg)
+ return msg;
// add one more segment for each following row
for (i = 2; msg == MAL_SUCCEED && i < size; i++) {
msg = wkbExtractPointToCoordSeq(&outCoordSeq, inWKB_array[i],
i);
+ if (msg)
+ return msg;
}
if ((outGeometry = GEOSGeom_createLineString_r(geoshandle,
outCoordSeq)) == NULL) {
msg = createException(MAL, "geom.MakeLine", SQLSTATE(38000)
"Geos operation GEOSGeom_createLineString failed");
+ return msg;
}
*outWKB = geos2wkb(outGeometry);
GEOSGeom_destroy_r(geoshandle, outGeometry);
@@ -3491,7 +3497,7 @@ wkbMakeLineAggrSubGroupedCand(bat *outid
str msg = MAL_SUCCEED;
oid min, max;
- BUN ngrp;
+ BUN ngrp = 0;
struct canditer ci;
oid lastGrp = -1;
@@ -3590,9 +3596,6 @@ wkbMakeLineAggrSubGroupedCand(bat *outid
if (BUNappendmulti(out, lines, ngrp, false) != GDK_SUCCEED) {
msg = createException(MAL, "geom.Union", SQLSTATE(38000)
"BUNappend operation failed");
- for (BUN i = 0; i < ngrp; i++)
- GDKfree(lines[i]);
- GDKfree(lines);
bat_iterator_end(&bi);
goto free;
}
@@ -3611,6 +3614,11 @@ wkbMakeLineAggrSubGroupedCand(bat *outid
BBPunfix(s->batCacheid);
return MAL_SUCCEED;
free:
+ if (lines) {
+ for (BUN i = 0; i < ngrp; i++)
+ GDKfree(lines[i]);
+ GDKfree(lines);
+ }
if (b)
BBPunfix(b->batCacheid);
if (g)
_______________________________________________
checkin-list mailing list -- [email protected]
To unsubscribe send an email to [email protected]
