[REPORT] Static analys warnings
1. Warning: the right operand to | always evaluates to 0
src/include/storage/bufpage.h
#define PAI_OVERWRITE (1 << 0)
#define PAI_IS_HEAP (1 << 1)
#define PageAddItem(page, item, size, offsetNumber, overwrite, is_heap) \
PageAddItemExtended(page, item, size, offsetNumber, \
((overwrite) ? PAI_OVERWRITE : 0) | \
((is_heap) ? PAI_IS_HEAP : 0))
Typo | should be ||:
((overwrite) ? PAI_OVERWRITE : 0) || \
((is_heap) ? PAI_IS_HEAP : 0))
regards,
Ranier Vilela
On 2020-05-03 17:05:54 -0300, Ranier Vilela wrote:
1. Warning: the right operand to | always evaluates to 0
src/include/storage/bufpage.h
#define PAI_OVERWRITE (1 << 0)
#define PAI_IS_HEAP (1 << 1)#define PageAddItem(page, item, size, offsetNumber, overwrite, is_heap) \
PageAddItemExtended(page, item, size, offsetNumber, \
((overwrite) ? PAI_OVERWRITE : 0) | \
((is_heap) ? PAI_IS_HEAP : 0))Typo | should be ||:
((overwrite) ? PAI_OVERWRITE : 0) || \
((is_heap) ? PAI_IS_HEAP : 0))
Definitely not. PageAddItemExtended's flags argument is not a boolean,
it's a flag bitmask. It'd entirely break the semantics to make the
change you suggest.
Nor do I know what this warning is about, because clearly in the general
case the right argument to the | does not generally evaluate to 0. I
guess this about a particular use of the macro (with a constant
argument), rather than the macro itself.
Fix possible overflow when converting, possible negative number to uint16.
postingoff can be -1,when converts to uint16, overflow can raise.
Otherwise, truncation can be occurs, losing precision, from int (31 bits)
to uint16 (15 bits)
There is a little confusion in the parameters of some functions in this
file, postigoff is declared as int, other declared as uint16.
src/backend/access/nbtree/nbtinsert.c
static void _bt_insertonpg(Relation rel, BTScanInsert itup_key,
Buffer buf,
Buffer cbuf,
BTStack stack,
IndexTuple itup,
Size itemsz,
OffsetNumber newitemoff,
int postingoff, // INT
bool split_only_page);
static Buffer _bt_split(Relation rel, BTScanInsert itup_key, Buffer buf,
Buffer cbuf, OffsetNumber newitemoff, Size newitemsz,
IndexTuple newitem, IndexTuple orignewitem,
IndexTuple nposting, uint16 postingoff); // UINT16
regards,
Ranier Vilela
Attachments:
fix_possible_overflow_postingoff.patchapplication/octet-stream; name=fix_possible_overflow_postingoff.patchDownload
diff --git a/src/backend/access/nbtree/nbtinsert.c b/src/backend/access/nbtree/nbtinsert.c
index 1c1d4a4bc9..c2b587e099 100644
--- a/src/backend/access/nbtree/nbtinsert.c
+++ b/src/backend/access/nbtree/nbtinsert.c
@@ -1289,7 +1289,6 @@ _bt_insertonpg(Relation rel,
xl_btree_metadata xlmeta;
uint8 xlinfo;
XLogRecPtr recptr;
- uint16 upostingoff;
xlrec.offnum = newitemoff;
@@ -1340,13 +1339,15 @@ _bt_insertonpg(Relation rel,
}
XLogRegisterBuffer(0, buf, REGBUF_STANDARD);
- if (postingoff == 0)
+ if (postingoff <= 0)
{
/* Just log itup from caller */
XLogRegisterBufData(0, (char *) itup, IndexTupleSize(itup));
}
else
{
+ uint16 upostingoff;
+
/*
* Insert with posting list split (XLOG_BTREE_INSERT_POST
* record) case.