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