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.

diff --git a/contrib/bloom/blinsert.c b/contrib/bloom/blinsert.c
index 9e6678087ca8140a99012ada00d18811e9f38497..6eecb12187dc09f7c6564cc82baa95d07f38e02f 100644 (file)
--- 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 fb0bc07f2846d9b58b70f23ac7cea8c708985ffa..8f3881d844665b0ec1c60ca91a8b28e2574d8fd6 100644 (file)
--- 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 d156e88669ab14af3b2b08aaaedd5deb2beb9b52..6e3cb84bb11d09935bba41c733221c3282f37fdc 100644 (file)
--- 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 */