mirrors/postgres
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Clean up some stuff in new contrib/bloom module.

Browse Source 
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.
This commit is contained in:
Tom Lane 2016-04-03 14:17:20 -04:00
parent 3e4b7d8798
commit a9284849b4
3 changed files with 15 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
contrib/bloom/blinsert.c
View File

@ -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;

4
contrib/bloom/bloom.h
View File

@ -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)

7
contrib/bloom/blscan.c
View File

@ -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 */