Thread: Replace offnum++ by OffsetNumberNext
This is the patch replace offnum++ by OffsetNumberNext.
According to off.h, OffsetNumberNext is the macro prepared to
disambiguate the different manipulations on OffsetNumbers.
But, increment operator was used in some places instead of the macro.
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
TEL (03)5860-5115
FAX (03)5463-5490
? patch.diff
Index: src/backend/access/heap/pruneheap.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/access/heap/pruneheap.c,v
retrieving revision 1.9
diff -c -r1.9 pruneheap.c
*** src/backend/access/heap/pruneheap.c 26 Mar 2008 21:10:37 -0000 1.9
--- src/backend/access/heap/pruneheap.c 4 Apr 2008 14:34:19 -0000
***************
*** 789,795 ****
MemSet(root_offsets, 0, MaxHeapTuplesPerPage * sizeof(OffsetNumber));
maxoff = PageGetMaxOffsetNumber(page);
! for (offnum = FirstOffsetNumber; offnum <= maxoff; offnum++)
{
ItemId lp = PageGetItemId(page, offnum);
HeapTupleHeader htup;
--- 789,795 ----
MemSet(root_offsets, 0, MaxHeapTuplesPerPage * sizeof(OffsetNumber));
maxoff = PageGetMaxOffsetNumber(page);
! for (offnum = FirstOffsetNumber; offnum <= maxoff; offnum = OffsetNumberNext(offnum))
{
ItemId lp = PageGetItemId(page, offnum);
HeapTupleHeader htup;
Index: src/backend/executor/nodeBitmapHeapscan.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/executor/nodeBitmapHeapscan.c,v
retrieving revision 1.25
diff -c -r1.25 nodeBitmapHeapscan.c
*** src/backend/executor/nodeBitmapHeapscan.c 26 Mar 2008 21:10:38 -0000 1.25
--- src/backend/executor/nodeBitmapHeapscan.c 4 Apr 2008 14:34:19 -0000
***************
*** 301,307 ****
OffsetNumber maxoff = PageGetMaxOffsetNumber(dp);
OffsetNumber offnum;
! for (offnum = FirstOffsetNumber; offnum <= maxoff; offnum++)
{
ItemId lp;
HeapTupleData loctup;
--- 301,307 ----
OffsetNumber maxoff = PageGetMaxOffsetNumber(dp);
OffsetNumber offnum;
! for (offnum = FirstOffsetNumber; offnum <= maxoff; offnum = OffsetNumberNext(offnum))
{
ItemId lp;
HeapTupleData loctup;
Index: src/backend/storage/page/bufpage.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/storage/page/bufpage.c,v
retrieving revision 1.78
diff -c -r1.78 bufpage.c
*** src/backend/storage/page/bufpage.c 10 Feb 2008 20:39:08 -0000 1.78
--- src/backend/storage/page/bufpage.c 4 Apr 2008 14:34:19 -0000
***************
*** 533,539 ****
* Since this is just a hint, we must confirm that there is
* indeed a free line pointer
*/
! for (offnum = FirstOffsetNumber; offnum <= nline; offnum++)
{
ItemId lp = PageGetItemId(page, offnum);
--- 533,539 ----
* Since this is just a hint, we must confirm that there is
* indeed a free line pointer
*/
! for (offnum = FirstOffsetNumber; offnum <= nline; offnum = OffsetNumberNext(offnum))
{
ItemId lp = PageGetItemId(page, offnum);
***************
*** 736,742 ****
totallen = 0;
nused = 0;
nextitm = 0;
! for (offnum = 1; offnum <= nline; offnum++)
{
lp = PageGetItemId(page, offnum);
Assert(ItemIdHasStorage(lp));
--- 736,742 ----
totallen = 0;
nused = 0;
nextitm = 0;
! for (offnum = FirstOffsetNumber; offnum <= nline; offnum = OffsetNumberNext(offnum))
{
lp = PageGetItemId(page, offnum);
Assert(ItemIdHasStorage(lp));
Fujii Masao <fujii.masao@oss.ntt.co.jp> writes:
> This is the patch replace offnum++ by OffsetNumberNext.
> According to off.h, OffsetNumberNext is the macro prepared to
> disambiguate the different manipulations on OffsetNumbers.
> But, increment operator was used in some places instead of the macro.
I wonder if we shouldn't go the other way, ie, use ++ everywhere.
OffsetNumberNext seems like just useless obscurantism ...
regards, tom lane
Tom Lane wrote: > Fujii Masao <fujii.masao@oss.ntt.co.jp> writes: > > This is the patch replace offnum++ by OffsetNumberNext. > > According to off.h, OffsetNumberNext is the macro prepared to > > disambiguate the different manipulations on OffsetNumbers. > > But, increment operator was used in some places instead of the macro. > > I wonder if we shouldn't go the other way, ie, use ++ everywhere. > OffsetNumberNext seems like just useless obscurantism ... The only value I saw to OffsetNumberNext was the fact is does int16 increment, with casting. There is also the comment: * Increments/decrements the argument. These macros look pointless * but they help us disambiguate the different manipulations on * OffsetNumbers (e.g., sometimes we subtract one from an * OffsetNumber to move back, and sometimes we do so to form a * real C array index). -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Patch applied. Thanks.
---------------------------------------------------------------------------
Fujii Masao wrote:
> This is the patch replace offnum++ by OffsetNumberNext.
>
> According to off.h, OffsetNumberNext is the macro prepared to
> disambiguate the different manipulations on OffsetNumbers.
> But, increment operator was used in some places instead of the macro.
>
> --
> Fujii Masao
> NIPPON TELEGRAPH AND TELEPHONE CORPORATION
> NTT Open Source Software Center
> TEL (03)5860-5115
> FAX (03)5463-5490
> ? patch.diff
> Index: src/backend/access/heap/pruneheap.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/backend/access/heap/pruneheap.c,v
> retrieving revision 1.9
> diff -c -r1.9 pruneheap.c
> *** src/backend/access/heap/pruneheap.c 26 Mar 2008 21:10:37 -0000 1.9
> --- src/backend/access/heap/pruneheap.c 4 Apr 2008 14:34:19 -0000
> ***************
> *** 789,795 ****
> MemSet(root_offsets, 0, MaxHeapTuplesPerPage * sizeof(OffsetNumber));
>
> maxoff = PageGetMaxOffsetNumber(page);
> ! for (offnum = FirstOffsetNumber; offnum <= maxoff; offnum++)
> {
> ItemId lp = PageGetItemId(page, offnum);
> HeapTupleHeader htup;
> --- 789,795 ----
> MemSet(root_offsets, 0, MaxHeapTuplesPerPage * sizeof(OffsetNumber));
>
> maxoff = PageGetMaxOffsetNumber(page);
> ! for (offnum = FirstOffsetNumber; offnum <= maxoff; offnum = OffsetNumberNext(offnum))
> {
> ItemId lp = PageGetItemId(page, offnum);
> HeapTupleHeader htup;
> Index: src/backend/executor/nodeBitmapHeapscan.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/backend/executor/nodeBitmapHeapscan.c,v
> retrieving revision 1.25
> diff -c -r1.25 nodeBitmapHeapscan.c
> *** src/backend/executor/nodeBitmapHeapscan.c 26 Mar 2008 21:10:38 -0000 1.25
> --- src/backend/executor/nodeBitmapHeapscan.c 4 Apr 2008 14:34:19 -0000
> ***************
> *** 301,307 ****
> OffsetNumber maxoff = PageGetMaxOffsetNumber(dp);
> OffsetNumber offnum;
>
> ! for (offnum = FirstOffsetNumber; offnum <= maxoff; offnum++)
> {
> ItemId lp;
> HeapTupleData loctup;
> --- 301,307 ----
> OffsetNumber maxoff = PageGetMaxOffsetNumber(dp);
> OffsetNumber offnum;
>
> ! for (offnum = FirstOffsetNumber; offnum <= maxoff; offnum = OffsetNumberNext(offnum))
> {
> ItemId lp;
> HeapTupleData loctup;
> Index: src/backend/storage/page/bufpage.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/backend/storage/page/bufpage.c,v
> retrieving revision 1.78
> diff -c -r1.78 bufpage.c
> *** src/backend/storage/page/bufpage.c 10 Feb 2008 20:39:08 -0000 1.78
> --- src/backend/storage/page/bufpage.c 4 Apr 2008 14:34:19 -0000
> ***************
> *** 533,539 ****
> * Since this is just a hint, we must confirm that there is
> * indeed a free line pointer
> */
> ! for (offnum = FirstOffsetNumber; offnum <= nline; offnum++)
> {
> ItemId lp = PageGetItemId(page, offnum);
>
> --- 533,539 ----
> * Since this is just a hint, we must confirm that there is
> * indeed a free line pointer
> */
> ! for (offnum = FirstOffsetNumber; offnum <= nline; offnum = OffsetNumberNext(offnum))
> {
> ItemId lp = PageGetItemId(page, offnum);
>
> ***************
> *** 736,742 ****
> totallen = 0;
> nused = 0;
> nextitm = 0;
> ! for (offnum = 1; offnum <= nline; offnum++)
> {
> lp = PageGetItemId(page, offnum);
> Assert(ItemIdHasStorage(lp));
> --- 736,742 ----
> totallen = 0;
> nused = 0;
> nextitm = 0;
> ! for (offnum = FirstOffsetNumber; offnum <= nline; offnum = OffsetNumberNext(offnum))
> {
> lp = PageGetItemId(page, offnum);
> Assert(ItemIdHasStorage(lp));
>
>
> --
> Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-patches
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +