aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2016-04-03 14:17:20 -0400
committerTom Lane <tgl@sss.pgh.pa.us>2016-04-03 14:17:23 -0400
commita9284849b48b04fa2836aaf704659974c13e610d (patch)
tree8e3f1667e6b73a8fe636ed74ae847c23565216b7
parent3e4b7d87988f0835f137f15f5c1a40598dd21f3d (diff)
downloadpostgresql-a9284849b48b04fa2836aaf704659974c13e610d.tar.gz
postgresql-a9284849b48b04fa2836aaf704659974c13e610d.zip
Clean up some stuff in new contrib/bloom module.
Coverity complained about implicit sign-extension in the BloomPageGetFreeSpace macro, probably because sizeOfBloomTuple isn't wide enough for size calculations. No overflow is really possible as long as maxoff and sizeOfBloomTuple are small enough to represent a realistic situation, but it seems like a good idea to declare sizeOfBloomTuple as Size not int32. Add missing check on BloomPageAddItem() result, again from Coverity. Avoid core dump due to not allocating so->sign array when scan->numberOfKeys is zero. Also thanks to Coverity. Use FLEXIBLE_ARRAY_MEMBER rather than declaring an array as size 1 when it isn't necessarily. Very minor beautification of related code. Unfortunately, none of the Coverity-detected mistakes look like they could account for the remaining buildfarm unhappiness with this module. It's barely possible that the FLEXIBLE_ARRAY_MEMBER mistake does account for that, if it's enabling bogus compiler optimizations; but I'm not terribly optimistic. We probably still have bugs to find here.
-rw-r--r--contrib/bloom/blinsert.c11
-rw-r--r--contrib/bloom/bloom.h4
-rw-r--r--contrib/bloom/blscan.c7
3 files changed, 15 insertions, 7 deletions
diff --git a/contrib/bloom/blinsert.c b/contrib/bloom/blinsert.c
index 9e6678087ca..6eecb12187d 100644
--- a/contrib/bloom/blinsert.c
+++ b/contrib/bloom/blinsert.c
@@ -96,10 +96,10 @@ bloomBuildCallback(Relation index, HeapTuple htup, Datum *values,
initCachedPage(buildstate);
- if (BloomPageAddItem(&buildstate->blstate, buildstate->data, itup) == false)
+ if (!BloomPageAddItem(&buildstate->blstate, buildstate->data, itup))
{
/* We shouldn't be here since we're inserting to the empty page */
- elog(ERROR, "can not add new tuple");
+ elog(ERROR, "could not add new bloom tuple to empty page");
}
}
@@ -298,7 +298,12 @@ blinsert(Relation index, Datum *values, bool *isnull,
metaData = BloomPageGetMeta(metaPage);
page = GenericXLogRegister(state, buffer, true);
BloomInitPage(page, 0);
- BloomPageAddItem(&blstate, page, itup);
+
+ if (!BloomPageAddItem(&blstate, page, itup))
+ {
+ /* We shouldn't be here since we're inserting to the empty page */
+ elog(ERROR, "could not add new bloom tuple to empty page");
+ }
metaData->nStart = 0;
metaData->nEnd = 1;
diff --git a/contrib/bloom/bloom.h b/contrib/bloom/bloom.h
index fb0bc07f284..8f3881d8446 100644
--- a/contrib/bloom/bloom.h
+++ b/contrib/bloom/bloom.h
@@ -118,7 +118,7 @@ typedef struct BloomState
* sizeOfBloomTuple is index's specific, and it depends on reloptions, so
* precompute it
*/
- int32 sizeOfBloomTuple;
+ Size sizeOfBloomTuple;
} BloomState;
#define BloomPageGetFreeSpace(state, page) \
@@ -134,7 +134,7 @@ typedef uint16 SignType;
typedef struct BloomTuple
{
ItemPointerData heapPtr;
- SignType sign[1];
+ SignType sign[FLEXIBLE_ARRAY_MEMBER];
} BloomTuple;
#define BLOOMTUPLEHDRSZ offsetof(BloomTuple, sign)
diff --git a/contrib/bloom/blscan.c b/contrib/bloom/blscan.c
index d156e88669a..6e3cb84bb11 100644
--- a/contrib/bloom/blscan.c
+++ b/contrib/bloom/blscan.c
@@ -94,7 +94,7 @@ blgetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
BufferAccessStrategy bas;
BloomScanOpaque so = (BloomScanOpaque) scan->opaque;
- if (so->sign == NULL && scan->numberOfKeys > 0)
+ if (so->sign == NULL)
{
/* New search: have to calculate search signature */
ScanKey skey = scan->keyData;
@@ -151,10 +151,13 @@ blgetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
bool res = true;
/* Check index signature with scan signature */
- for (i = 0; res && i < so->state.opts->bloomLength; i++)
+ for (i = 0; i < so->state.opts->bloomLength; i++)
{
if ((itup->sign[i] & so->sign[i]) != so->sign[i])
+ {
res = false;
+ break;
+ }
}
/* Add matching tuples to bitmap */