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 -- checkin-list@monetdb.org
To unsubscribe send an email to checkin-list-le...@monetdb.org

Reply via email to