From: Tom Lane Date: Tue, 6 Aug 2002 19:41:23 +0000 (+0000) Subject: Still more paranoia in PageAddItem: disallow specification of an item X-Git-Tag: REL7_3~995 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ba053de197eaec975e611b2969526363915e58c2;p=postgresql Still more paranoia in PageAddItem: disallow specification of an item offset past the last-used-item-plus-one, since that would result in leaving uninitialized holes in the item pointer array. AFAICT the only place that was depending on this was btree index build, which was being cavalier about when to fill in the P_HIKEY pointer; easily fixed. Also a small performance improvement: shuffle itemid's by means of memmove, not a one-at-a-time loop. --- diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c index 80d56f0451..b6412e6d17 100644 --- a/src/backend/access/nbtree/nbtsort.c +++ b/src/backend/access/nbtree/nbtsort.c @@ -35,7 +35,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/access/nbtree/nbtsort.c,v 1.65 2002/07/02 05:48:44 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/access/nbtree/nbtsort.c,v 1.66 2002/08/06 19:41:23 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -187,10 +187,17 @@ _bt_blnewpage(Relation index, Buffer *buf, Page *page, int flags) *buf = _bt_getbuf(index, P_NEW, BT_WRITE); *page = BufferGetPage(*buf); + + /* Zero the page and set up standard page header info */ _bt_pageinit(*page, BufferGetPageSize(*buf)); + + /* Initialize BT opaque state */ opaque = (BTPageOpaque) PageGetSpecialPointer(*page); opaque->btpo_prev = opaque->btpo_next = P_NONE; opaque->btpo_flags = flags; + + /* Make the P_HIKEY line pointer appear allocated */ + ((PageHeader) *page)->pd_lower += sizeof(ItemIdData); } /* diff --git a/src/backend/storage/page/bufpage.c b/src/backend/storage/page/bufpage.c index e6abf3b41e..ca8186ccda 100644 --- a/src/backend/storage/page/bufpage.c +++ b/src/backend/storage/page/bufpage.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/storage/page/bufpage.c,v 1.46 2002/07/02 05:48:44 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/storage/page/bufpage.c,v 1.47 2002/08/06 19:41:23 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -73,7 +73,6 @@ PageAddItem(Page page, ItemIdFlags flags) { PageHeader phdr = (PageHeader) page; - int i; Size alignedSize; int lower; int upper; @@ -91,24 +90,20 @@ PageAddItem(Page page, phdr->pd_lower > phdr->pd_upper || phdr->pd_upper > phdr->pd_special || phdr->pd_special > BLCKSZ) - elog(ERROR, "PageAddItem: corrupted page pointers: lower = %u, upper = %u, special = %u", + elog(PANIC, "PageAddItem: corrupted page pointers: lower = %u, upper = %u, special = %u", phdr->pd_lower, phdr->pd_upper, phdr->pd_special); /* - * Find first unallocated offsetNumber + * Select offsetNumber to place the new item at */ limit = OffsetNumberNext(PageGetMaxOffsetNumber(page)); /* was offsetNumber passed in? */ if (OffsetNumberIsValid(offsetNumber)) { + /* yes, check it */ if (overwritemode) { - if (offsetNumber > limit) - { - elog(WARNING, "PageAddItem: tried overwrite after maxoff"); - return InvalidOffsetNumber; - } if (offsetNumber < limit) { itemId = PageGetItemId(phdr, offsetNumber); @@ -122,16 +117,13 @@ PageAddItem(Page page, } else { - /* - * Don't actually do the shuffle till we've checked free - * space! - */ - needshuffle = true; /* need to increase "lower" */ + if (offsetNumber < limit) + needshuffle = true; /* need to move existing linp's */ } } else { - /* offsetNumber was not passed in, so find one */ + /* offsetNumber was not passed in, so find a free slot */ /* look for "recyclable" (unused & deallocated) ItemId */ for (offsetNumber = 1; offsetNumber < limit; offsetNumber++) { @@ -140,6 +132,13 @@ PageAddItem(Page page, ((*itemId).lp_len == 0)) break; } + /* if no free slot, we'll put it at limit (1st open slot) */ + } + + if (offsetNumber > limit) + { + elog(WARNING, "PageAddItem: specified offset after maxoff"); + return InvalidOffsetNumber; } /* @@ -148,9 +147,7 @@ PageAddItem(Page page, * Note: do arithmetic as signed ints, to avoid mistakes if, say, * alignedSize > pd_upper. */ - if (offsetNumber > limit) - lower = (char *) PageGetItemId(phdr, offsetNumber + 1) - (char *) page; - else if (offsetNumber == limit || needshuffle) + if (offsetNumber == limit || needshuffle) lower = phdr->pd_lower + sizeof(ItemIdData); else lower = phdr->pd_lower; @@ -166,27 +163,21 @@ PageAddItem(Page page, * OK to insert the item. First, shuffle the existing pointers if * needed. */ - if (needshuffle) - { - /* shuffle ItemId's (Do the PageManager Shuffle...) */ - for (i = (int) limit - 1; i >= (int) offsetNumber; i--) - { - ItemId fromitemId, - toitemId; + itemId = PageGetItemId(phdr, offsetNumber); - fromitemId = PageGetItemId(phdr, i); - toitemId = PageGetItemId(phdr, i + 1); - *toitemId = *fromitemId; - } - } + if (needshuffle) + memmove(itemId + 1, itemId, + (limit - offsetNumber) * sizeof(ItemIdData)); - itemId = PageGetItemId(phdr, offsetNumber); + /* set the item pointer */ (*itemId).lp_off = upper; (*itemId).lp_len = size; (*itemId).lp_flags = flags; - memmove((char *) page + upper, item, size); + /* copy the item's data onto the page */ + memcpy((char *) page + upper, item, size); + /* adjust page header */ phdr->pd_lower = (LocationIndex) lower; phdr->pd_upper = (LocationIndex) upper; @@ -235,7 +226,7 @@ PageRestoreTempPage(Page tempPage, Page oldPage) Size pageSize; pageSize = PageGetPageSize(tempPage); - memmove((char *) oldPage, (char *) tempPage, pageSize); + memcpy((char *) oldPage, (char *) tempPage, pageSize); pfree(tempPage); } @@ -457,9 +448,10 @@ PageIndexTupleDelete(Page page, OffsetNumber offnum) nbytes = phdr->pd_lower - ((char *) &phdr->pd_linp[offidx + 1] - (char *) phdr); - memmove((char *) &(phdr->pd_linp[offidx]), - (char *) &(phdr->pd_linp[offidx + 1]), - nbytes); + if (nbytes > 0) + memmove((char *) &(phdr->pd_linp[offidx]), + (char *) &(phdr->pd_linp[offidx + 1]), + nbytes); /* * Now move everything between the old upper bound (beginning of tuple