From 0a208287a9cf1ffa5585ad835c15b5eae4645e8a Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Mon, 5 Jan 2015 14:02:00 +0900 Subject: [PATCH] Fix XLOG reader allocation assuming that palloc can be NULL 2c03216 has updated allocate_recordbuf to use palloc instead of malloc when allocating a record buffer, falsing the assumption taken by some code paths expecting an OOM and reporting a dedicated error message if a NULL pointer was created at allocation, something that cannot happen with palloc because it always fails in case of an OOM. Hence remove those checks, and use at the same time repalloc_huge, now needed as well on frontend side by at least pg_xlogdump to cover as well the fact that palloc cannot allocate more than 1GB. --- contrib/pg_xlogdump/pg_xlogdump.c | 2 -- src/backend/access/transam/xlog.c | 8 +------- src/backend/access/transam/xlogreader.c | 33 ++++++++++----------------------- src/common/fe_memutils.c | 6 ++++++ src/include/common/fe_memutils.h | 1 + 5 files changed, 18 insertions(+), 32 deletions(-) diff --git a/contrib/pg_xlogdump/pg_xlogdump.c b/contrib/pg_xlogdump/pg_xlogdump.c index 9f05e25..762269e 100644 --- a/contrib/pg_xlogdump/pg_xlogdump.c +++ b/contrib/pg_xlogdump/pg_xlogdump.c @@ -916,8 +916,6 @@ main(int argc, char **argv) /* we have everything we need, start reading */ xlogreader_state = XLogReaderAllocate(XLogDumpReadPage, &private); - if (!xlogreader_state) - fatal_error("out of memory"); /* first find a valid recptr to start from */ first_record = XLogFindNextRecord(xlogreader_state, private.startptr); diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index e5dddd4..7d2ca49 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -1063,8 +1063,7 @@ XLogInsertRecord(XLogRecData *rdata, XLogRecPtr fpw_lsn) if (!debug_reader) debug_reader = XLogReaderAllocate(NULL, NULL); - if (!debug_reader || - !DecodeXLogRecord(debug_reader, (XLogRecord *) recordBuf.data, + if (!DecodeXLogRecord(debug_reader, (XLogRecord *) recordBuf.data, &errormsg)) { appendStringInfo(&buf, "error decoding record: %s", @@ -5785,11 +5784,6 @@ StartupXLOG(void) /* Set up XLOG reader facility */ MemSet(&private, 0, sizeof(XLogPageReadPrivate)); xlogreader = XLogReaderAllocate(&XLogPageRead, &private); - if (!xlogreader) - ereport(ERROR, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"), - errdetail("Failed while allocating an XLog reading processor."))); xlogreader->system_identifier = ControlFile->system_identifier; if (read_backup_label(&checkPointLoc, &backupEndRequired, diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index 67d6223..71701ff 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -21,7 +21,7 @@ #include "access/xlogreader.h" #include "catalog/pg_control.h" -static bool allocate_recordbuf(XLogReaderState *state, uint32 reclength); +static void allocate_recordbuf(XLogReaderState *state, uint32 reclength); static bool ValidXLogPageHeader(XLogReaderState *state, XLogRecPtr recptr, XLogPageHeader hdr); @@ -94,13 +94,7 @@ XLogReaderAllocate(XLogPageReadCB pagereadfunc, void *private_data) * Allocate an initial readRecordBuf of minimal size, which can later be * enlarged if necessary. */ - if (!allocate_recordbuf(state, 0)) - { - pfree(state->errormsg_buf); - pfree(state->readBuf); - pfree(state); - return NULL; - } + allocate_recordbuf(state, 0); return state; } @@ -130,7 +124,6 @@ XLogReaderFree(XLogReaderState *state) /* * Allocate readRecordBuf to fit a record of at least the given length. - * Returns true if successful, false if out of memory. * * readRecordBufSize is set to the new buffer size. * @@ -139,7 +132,7 @@ XLogReaderFree(XLogReaderState *state) * with. (That is enough for all "normal" records, but very large commit or * abort records might need more space.) */ -static bool +static void allocate_recordbuf(XLogReaderState *state, uint32 reclength) { uint32 newSize = reclength; @@ -147,11 +140,12 @@ allocate_recordbuf(XLogReaderState *state, uint32 reclength) newSize += XLOG_BLCKSZ - (newSize % XLOG_BLCKSZ); newSize = Max(newSize, 5 * Max(BLCKSZ, XLOG_BLCKSZ)); - if (state->readRecordBuf) - pfree(state->readRecordBuf); - state->readRecordBuf = (char *) palloc(newSize); + if (state->readRecordBuf == NULL) + state->readRecordBuf = (char *) palloc(newSize); + else + state->readRecordBuf = + (char *) repalloc_huge(state->readRecordBuf, newSize); state->readRecordBufSize = newSize; - return true; } /* @@ -302,15 +296,8 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg) /* * Enlarge readRecordBuf as needed. */ - if (total_len > state->readRecordBufSize && - !allocate_recordbuf(state, total_len)) - { - /* We treat this as a "bogus data" condition */ - report_invalid_record(state, "record length %u at %X/%X too long", - total_len, - (uint32) (RecPtr >> 32), (uint32) RecPtr); - goto err; - } + if (total_len > state->readRecordBufSize) + allocate_recordbuf(state, total_len); len = XLOG_BLCKSZ - RecPtr % XLOG_BLCKSZ; if (total_len > len) diff --git a/src/common/fe_memutils.c b/src/common/fe_memutils.c index f90ae64..beffe48 100644 --- a/src/common/fe_memutils.c +++ b/src/common/fe_memutils.c @@ -126,3 +126,9 @@ repalloc(void *pointer, Size size) { return pg_realloc(pointer, size); } + +void * +repalloc_huge(void *pointer, Size size) +{ + return pg_realloc(pointer, size); +} diff --git a/src/include/common/fe_memutils.h b/src/include/common/fe_memutils.h index 61c1b6f..610a854 100644 --- a/src/include/common/fe_memutils.h +++ b/src/include/common/fe_memutils.h @@ -21,6 +21,7 @@ extern char *pstrdup(const char *in); extern void *palloc(Size size); extern void *palloc0(Size size); extern void *repalloc(void *pointer, Size size); +extern void *repalloc_huge(void *pointer, Size size); extern void pfree(void *pointer); /* sprintf into a palloc'd buffer --- these are in psprintf.c */ -- 2.2.1