Skip to content

Commit bd037dc

Browse files
tglsfdctglsfdc
committed
Make XLogRecGetBlockTag() throw error if there's no such block.
All but a few existing callers assume without checking that this function succeeds. While it probably will, that's a poor excuse for not checking. Let's make it return void and instead throw an error if it doesn't find the block reference. Callers that actually need to handle the no-such-block case must now use the underlying function XLogRecGetBlockTagExtended. In addition to being a bit less error-prone, this should also serve to suppress some Coverity complaints about XLogRecGetBlockRefInfo. While at it, clean up some inconsistency about use of the XLogRecHasBlockRef macro: make XLogRecGetBlockTagExtended use that instead of open-coding the same condition, and avoid calling XLogRecHasBlockRef twice in relevant code paths. (That is, calling XLogRecHasBlockRef followed by XLogRecGetBlockTag is now deprecated: use XLogRecGetBlockTagExtended instead.) Patch HEAD only; this doesn't seem to have enough value to consider a back-branch API break. Discussion: https://postgr.es/m/425039.1649701221@sss.pgh.pa.us
1 parent 9debd12 commit bd037dc

File tree

9 files changed

+36
-23
lines changed

9 files changed

+36
-23
lines changed

‎src/backend/access/heap/heapam.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9363,7 +9363,7 @@ heap_xlog_update(XLogReaderState *record, bool hot_update)
93639363
oldtup.t_len = 0;
93649364

93659365
XLogRecGetBlockTag(record, 0, &rnode, NULL, &newblk);
9366-
if (XLogRecGetBlockTag(record, 1, NULL, NULL, &oldblk))
9366+
if (XLogRecGetBlockTagExtended(record, 1, NULL, NULL, &oldblk, NULL))
93679367
{
93689368
/* HOT updates are never done across pages */
93699369
Assert(!hot_update);

‎src/backend/access/nbtree/nbtxlog.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ btree_xlog_split(bool newitemonleft, XLogReaderState *record)
267267

268268
XLogRecGetBlockTag(record, 0, NULL, NULL, &origpagenumber);
269269
XLogRecGetBlockTag(record, 1, NULL, NULL, &rightpagenumber);
270-
if (!XLogRecGetBlockTag(record, 2, NULL, NULL, &spagenumber))
270+
if (!XLogRecGetBlockTagExtended(record, 2, NULL, NULL, &spagenumber, NULL))
271271
spagenumber = P_NONE;
272272

273273
/*

‎src/backend/access/rmgrdesc/xlogdesc.c

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -219,15 +219,14 @@ XLogRecGetBlockRefInfo(XLogReaderState *record, bool pretty,
219219

220220
for (block_id = 0; block_id <= XLogRecMaxBlockId(record); block_id++)
221221
{
222-
RelFileNode rnode = {InvalidOid, InvalidOid, InvalidOid};
223-
ForkNumber forknum = InvalidForkNumber;
224-
BlockNumber blk = InvalidBlockNumber;
222+
RelFileNode rnode;
223+
ForkNumber forknum;
224+
BlockNumber blk;
225225

226-
if (!XLogRecHasBlockRef(record, block_id))
226+
if (!XLogRecGetBlockTagExtended(record, block_id,
227+
&rnode, &forknum, &blk, NULL))
227228
continue;
228229

229-
XLogRecGetBlockTag(record, block_id, &rnode, &forknum, &blk);
230-
231230
if (detailed_format)
232231
{
233232
/* Get block references in detailed format. */

‎src/backend/access/transam/xlogreader.c

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@
3737
#include "miscadmin.h"
3838
#include "pgstat.h"
3939
#include "utils/memutils.h"
40+
#else
41+
#include "common/logging.h"
4042
#endif
4143

4244
static void report_invalid_record(XLogReaderState *state, const char *fmt,...)
@@ -1918,14 +1920,25 @@ DecodeXLogRecord(XLogReaderState *state,
19181920

19191921
/*
19201922
* Returns information about the block that a block reference refers to.
1921-
* See XLogRecGetBlockTagExtended().
1923+
*
1924+
* This is like XLogRecGetBlockTagExtended, except that the block reference
1925+
* must exist and there's no access to prefetch_buffer.
19221926
*/
1923-
bool
1927+
void
19241928
XLogRecGetBlockTag(XLogReaderState *record, uint8 block_id,
19251929
RelFileNode *rnode, ForkNumber *forknum, BlockNumber *blknum)
19261930
{
1927-
return XLogRecGetBlockTagExtended(record, block_id, rnode, forknum, blknum,
1928-
NULL);
1931+
if (!XLogRecGetBlockTagExtended(record, block_id, rnode, forknum, blknum,
1932+
NULL))
1933+
{
1934+
#ifndef FRONTEND
1935+
elog(ERROR, "failed to locate backup block with ID %d in WAL record",
1936+
block_id);
1937+
#else
1938+
pg_fatal("failed to locate backup block with ID %d in WAL record",
1939+
block_id);
1940+
#endif
1941+
}
19291942
}
19301943

19311944
/*
@@ -1944,8 +1957,7 @@ XLogRecGetBlockTagExtended(XLogReaderState *record, uint8 block_id,
19441957
{
19451958
DecodedBkpBlock *bkpb;
19461959

1947-
if (block_id > record->record->max_block_id ||
1948-
!record->record->blocks[block_id].in_use)
1960+
if (!XLogRecHasBlockRef(record, block_id))
19491961
return false;
19501962

19511963
bkpb = &record->record->blocks[block_id];

‎src/backend/access/transam/xlogrecovery.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2172,10 +2172,10 @@ xlog_block_info(StringInfo buf, XLogReaderState *record)
21722172
ForkNumber forknum;
21732173
BlockNumber blk;
21742174

2175-
if (!XLogRecHasBlockRef(record, block_id))
2175+
if (!XLogRecGetBlockTagExtended(record, block_id,
2176+
&rnode, &forknum, &blk, NULL))
21762177
continue;
21772178

2178-
XLogRecGetBlockTag(record, block_id, &rnode, &forknum, &blk);
21792179
if (forknum != MAIN_FORKNUM)
21802180
appendStringInfo(buf, "; blkref #%d: rel %u/%u/%u, fork %u, blk %u",
21812181
block_id,
@@ -2303,7 +2303,8 @@ verifyBackupPageConsistency(XLogReaderState *record)
23032303
Buffer buf;
23042304
Page page;
23052305

2306-
if (!XLogRecGetBlockTag(record, block_id, &rnode, &forknum, &blkno))
2306+
if (!XLogRecGetBlockTagExtended(record, block_id,
2307+
&rnode, &forknum, &blkno, NULL))
23072308
{
23082309
/*
23092310
* WAL record doesn't contain a block reference with the given id.

‎src/backend/access/transam/xlogutils.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,8 @@ XLogReadBufferForRedoExtended(XLogReaderState *record,
369369
&prefetch_buffer))
370370
{
371371
/* Caller specified a bogus block_id */
372-
elog(PANIC, "failed to locate backup block with ID %d", block_id);
372+
elog(PANIC, "failed to locate backup block with ID %d in WAL record",
373+
block_id);
373374
}
374375

375376
/*

‎src/bin/pg_rewind/parsexlog.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -450,7 +450,8 @@ extractPageInfo(XLogReaderState *record)
450450
ForkNumber forknum;
451451
BlockNumber blkno;
452452

453-
if (!XLogRecGetBlockTag(record, block_id, &rnode, &forknum, &blkno))
453+
if (!XLogRecGetBlockTagExtended(record, block_id,
454+
&rnode, &forknum, &blkno, NULL))
454455
continue;
455456

456457
/* We only care about the main fork; others are copied in toto */

‎src/bin/pg_waldump/pg_waldump.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -405,11 +405,10 @@ XLogRecordMatchesRelationBlock(XLogReaderState *record,
405405
ForkNumber forknum;
406406
BlockNumber blk;
407407

408-
if (!XLogRecHasBlockRef(record, block_id))
408+
if (!XLogRecGetBlockTagExtended(record, block_id,
409+
&rnode, &forknum, &blk, NULL))
409410
continue;
410411

411-
XLogRecGetBlockTag(record, block_id, &rnode, &forknum, &blk);
412-
413412
if ((matchFork == InvalidForkNumber || matchFork == forknum) &&
414413
(RelFileNodeEquals(matchRnode, emptyRelFileNode) ||
415414
RelFileNodeEquals(matchRnode, rnode)) &&

‎src/include/access/xlogreader.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,7 @@ extern FullTransactionId XLogRecGetFullXid(XLogReaderState *record);
429429

430430
extern bool RestoreBlockImage(XLogReaderState *record, uint8 block_id, char *page);
431431
extern char *XLogRecGetBlockData(XLogReaderState *record, uint8 block_id, Size *len);
432-
extern bool XLogRecGetBlockTag(XLogReaderState *record, uint8 block_id,
432+
extern void XLogRecGetBlockTag(XLogReaderState *record, uint8 block_id,
433433
RelFileNode *rnode, ForkNumber *forknum,
434434
BlockNumber *blknum);
435435
extern bool XLogRecGetBlockTagExtended(XLogReaderState *record, uint8 block_id,

0 commit comments

Comments
 (0)