Postgres Pro
Downloads
Home   >   mailing lists

Thread: [REPORT] Static analys warnings

[REPORT] Static analys warnings

From
Ranier Vilela
Date:
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

Re: [REPORT] Static analys warnings

From
Andres Freund
Date:
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.

Re: [REPORT] Static analys warnings

From
Ranier Vilela
Date:
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
Attachment