On Fri, Jun 10, 2011 at 21:07, Tom Lane <[email protected]> wrote:
> Magnus Hagander <[email protected]> writes:
>> I came across a situation today with a pretty bad crash of pg_dump,
>> due to not checking the return code from malloc(). When looking
>> through the code, it seems there are a *lot* of places in pg_dump that
>> doesn't check the malloc return code.
>
>> But we do have a pg_malloc() function in there - but from what I can
>> tell it's only used very sparsely?
>
>> Shouldn't we be using that one more or less everywhere
>
> Yup. Have at it.
Something along the line of this?
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
diff --git a/src/bin/pg_dump/Makefile b/src/bin/pg_dump/Makefile
index 8410af1..0737505 100644
--- a/src/bin/pg_dump/Makefile
+++ b/src/bin/pg_dump/Makefile
@@ -20,7 +20,7 @@ override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
OBJS= pg_backup_archiver.o pg_backup_db.o pg_backup_custom.o \
pg_backup_files.o pg_backup_null.o pg_backup_tar.o \
- pg_backup_directory.o dumputils.o compress_io.o $(WIN32RES)
+ pg_backup_directory.o pg_dump_alloc.o dumputils.o compress_io.o $(WIN32RES)
KEYWRDOBJS = keywords.o kwlookup.o
@@ -35,8 +35,8 @@ pg_dump: pg_dump.o common.o pg_dump_sort.o $(OBJS) $(KEYWRDOBJS) | submake-libpq
pg_restore: pg_restore.o $(OBJS) $(KEYWRDOBJS) | submake-libpq submake-libpgport
$(CC) $(CFLAGS) pg_restore.o $(KEYWRDOBJS) $(OBJS) $(libpq_pgport) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)
-pg_dumpall: pg_dumpall.o dumputils.o $(KEYWRDOBJS) | submake-libpq submake-libpgport
- $(CC) $(CFLAGS) pg_dumpall.o dumputils.o $(KEYWRDOBJS) $(WIN32RES) $(libpq_pgport) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)
+pg_dumpall: pg_dumpall.o dumputils.o pg_dump_alloc.o $(KEYWRDOBJS) | submake-libpq submake-libpgport
+ $(CC) $(CFLAGS) pg_dumpall.o dumputils.o pg_dump_alloc.o $(KEYWRDOBJS) $(WIN32RES) $(libpq_pgport) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)
install: all installdirs
$(INSTALL_PROGRAM) pg_dump$(X) '$(DESTDIR)$(bindir)'/pg_dump$(X)
diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c
index a631f64..b194f24 100644
--- a/src/bin/pg_dump/common.c
+++ b/src/bin/pg_dump/common.c
@@ -980,57 +980,3 @@ simple_string_list_member(SimpleStringList *list, const char *val)
}
-/*
- * Safer versions of some standard C library functions. If an
- * out-of-memory condition occurs, these functions will bail out
- * safely; therefore, their return value is guaranteed to be non-NULL.
- *
- * XXX need to refactor things so that these can be in a file that can be
- * shared by pg_dumpall and pg_restore as well as pg_dump.
- */
-
-char *
-pg_strdup(const char *string)
-{
- char *tmp;
-
- if (!string)
- exit_horribly(NULL, NULL, "cannot duplicate null pointer\n");
- tmp = strdup(string);
- if (!tmp)
- exit_horribly(NULL, NULL, "out of memory\n");
- return tmp;
-}
-
-void *
-pg_malloc(size_t size)
-{
- void *tmp;
-
- tmp = malloc(size);
- if (!tmp)
- exit_horribly(NULL, NULL, "out of memory\n");
- return tmp;
-}
-
-void *
-pg_calloc(size_t nmemb, size_t size)
-{
- void *tmp;
-
- tmp = calloc(nmemb, size);
- if (!tmp)
- exit_horribly(NULL, NULL, "out of memory\n");
- return tmp;
-}
-
-void *
-pg_realloc(void *ptr, size_t size)
-{
- void *tmp;
-
- tmp = realloc(ptr, size);
- if (!tmp)
- exit_horribly(NULL, NULL, "out of memory\n");
- return tmp;
-}
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 0e1037c..78b5e93 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -735,8 +735,6 @@ ArchiveEntry(Archive *AHX,
TocEntry *newToc;
newToc = (TocEntry *) calloc(1, sizeof(TocEntry));
- if (!newToc)
- die_horribly(AH, modulename, "out of memory\n");
AH->tocCount++;
if (dumpId > AH->maxDumpId)
@@ -1131,8 +1129,6 @@ archprintf(Archive *AH, const char *fmt,...)
free(p);
bSize *= 2;
p = (char *) malloc(bSize);
- if (p == NULL)
- exit_horribly(AH, modulename, "out of memory\n");
va_start(ap, fmt);
cnt = vsnprintf(p, bSize, fmt, ap);
va_end(ap);
@@ -1266,8 +1262,6 @@ ahprintf(ArchiveHandle *AH, const char *fmt,...)
free(p);
bSize *= 2;
p = (char *) malloc(bSize);
- if (p == NULL)
- die_horribly(AH, modulename, "out of memory\n");
va_start(ap, fmt);
cnt = vsnprintf(p, bSize, fmt, ap);
va_end(ap);
@@ -1736,8 +1730,6 @@ ReadStr(ArchiveHandle *AH)
else
{
buf = (char *) malloc(l + 1);
- if (!buf)
- die_horribly(AH, modulename, "out of memory\n");
if ((*AH->ReadBufPtr) (AH, (void *) buf, l) != l)
die_horribly(AH, modulename, "unexpected end of file\n");
@@ -1930,8 +1922,6 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
#endif
AH = (ArchiveHandle *) calloc(1, sizeof(ArchiveHandle));
- if (!AH)
- die_horribly(AH, modulename, "out of memory\n");
/* AH->debugLevel = 100; */
@@ -1976,8 +1966,6 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
AH->currWithOids = -1; /* force SET */
AH->toc = (TocEntry *) calloc(1, sizeof(TocEntry));
- if (!AH->toc)
- die_horribly(AH, modulename, "out of memory\n");
AH->toc->next = AH->toc;
AH->toc->prev = AH->toc;
@@ -4195,8 +4183,6 @@ CloneArchive(ArchiveHandle *AH)
/* Make a "flat" copy */
clone = (ArchiveHandle *) malloc(sizeof(ArchiveHandle));
- if (clone == NULL)
- die_horribly(AH, modulename, "out of memory\n");
memcpy(clone, AH, sizeof(ArchiveHandle));
/* Handle format-independent fields */
diff --git a/src/bin/pg_dump/pg_backup_custom.c b/src/bin/pg_dump/pg_backup_custom.c
index 01d5e37..e1635ae 100644
--- a/src/bin/pg_dump/pg_backup_custom.c
+++ b/src/bin/pg_dump/pg_backup_custom.c
@@ -128,15 +128,11 @@ InitArchiveFmt_Custom(ArchiveHandle *AH)
/* Set up a private area. */
ctx = (lclContext *) calloc(1, sizeof(lclContext));
- if (ctx == NULL)
- die_horribly(AH, modulename, "out of memory\n");
AH->formatData = (void *) ctx;
/* Initialize LO buffering */
AH->lo_buf_size = LOBBUFSIZE;
AH->lo_buf = (void *) malloc(LOBBUFSIZE);
- if (AH->lo_buf == NULL)
- die_horribly(AH, modulename, "out of memory\n");
ctx->filePos = 0;
@@ -771,8 +767,6 @@ _Clone(ArchiveHandle *AH)
lclContext *ctx = (lclContext *) AH->formatData;
AH->formatData = (lclContext *) malloc(sizeof(lclContext));
- if (AH->formatData == NULL)
- die_horribly(AH, modulename, "out of memory\n");
memcpy(AH->formatData, ctx, sizeof(lclContext));
ctx = (lclContext *) AH->formatData;
@@ -898,8 +892,6 @@ _CustomReadFunc(ArchiveHandle *AH, char **buf, size_t *buflen)
{
free(*buf);
*buf = (char *) malloc(blkLen);
- if (!(*buf))
- die_horribly(AH, modulename, "out of memory\n");
*buflen = blkLen;
}
diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c
index cc3882c..67deaa7 100644
--- a/src/bin/pg_dump/pg_backup_db.c
+++ b/src/bin/pg_dump/pg_backup_db.c
@@ -160,9 +160,6 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
const char **keywords = malloc(PARAMS_ARRAY_SIZE * sizeof(*keywords));
const char **values = malloc(PARAMS_ARRAY_SIZE * sizeof(*values));
- if (!keywords || !values)
- die_horribly(AH, modulename, "out of memory\n");
-
keywords[0] = "host";
values[0] = PQhost(AH->connection);
keywords[1] = "port";
@@ -267,9 +264,6 @@ ConnectDatabase(Archive *AHX,
const char **keywords = malloc(PARAMS_ARRAY_SIZE * sizeof(*keywords));
const char **values = malloc(PARAMS_ARRAY_SIZE * sizeof(*values));
- if (!keywords || !values)
- die_horribly(AH, modulename, "out of memory\n");
-
keywords[0] = "host";
values[0] = pghost;
keywords[1] = "port";
diff --git a/src/bin/pg_dump/pg_backup_directory.c b/src/bin/pg_dump/pg_backup_directory.c
index 111c3e8..b0bdf5f 100644
--- a/src/bin/pg_dump/pg_backup_directory.c
+++ b/src/bin/pg_dump/pg_backup_directory.c
@@ -127,8 +127,6 @@ InitArchiveFmt_Directory(ArchiveHandle *AH)
/* Set up our private context */
ctx = (lclContext *) calloc(1, sizeof(lclContext));
- if (ctx == NULL)
- die_horribly(AH, modulename, "out of memory\n");
AH->formatData = (void *) ctx;
ctx->dataFH = NULL;
@@ -137,8 +135,6 @@ InitArchiveFmt_Directory(ArchiveHandle *AH)
/* Initialize LO buffering */
AH->lo_buf_size = LOBBUFSIZE;
AH->lo_buf = (void *) malloc(LOBBUFSIZE);
- if (AH->lo_buf == NULL)
- die_horribly(AH, modulename, "out of memory\n");
/*
* Now open the TOC file
@@ -198,8 +194,6 @@ _ArchiveEntry(ArchiveHandle *AH, TocEntry *te)
char fn[MAXPGPATH];
tctx = (lclTocEntry *) calloc(1, sizeof(lclTocEntry));
- if (!tctx)
- die_horribly(AH, modulename, "out of memory\n");
if (te->dataDumper)
{
snprintf(fn, MAXPGPATH, "%d.dat", te->dumpId);
@@ -249,8 +243,6 @@ _ReadExtraToc(ArchiveHandle *AH, TocEntry *te)
if (tctx == NULL)
{
tctx = (lclTocEntry *) calloc(1, sizeof(lclTocEntry));
- if (!tctx)
- die_horribly(AH, modulename, "out of memory\n");
te->formatData = (void *) tctx;
}
@@ -357,8 +349,6 @@ _PrintFileData(ArchiveHandle *AH, char *filename, RestoreOptions *ropt)
filename, strerror(errno));
buf = malloc(ZLIB_OUT_SIZE);
- if (buf == NULL)
- die_horribly(NULL, modulename, "out of memory\n");
buflen = ZLIB_OUT_SIZE;
while ((cnt = cfread(buf, buflen, cfp)))
diff --git a/src/bin/pg_dump/pg_backup_files.c b/src/bin/pg_dump/pg_backup_files.c
index abc93b1..55dfab3 100644
--- a/src/bin/pg_dump/pg_backup_files.c
+++ b/src/bin/pg_dump/pg_backup_files.c
@@ -110,8 +110,6 @@ InitArchiveFmt_Files(ArchiveHandle *AH)
/* Initialize LO buffering */
AH->lo_buf_size = LOBBUFSIZE;
AH->lo_buf = (void *) malloc(LOBBUFSIZE);
- if (AH->lo_buf == NULL)
- die_horribly(AH, modulename, "out of memory\n");
/*
* Now open the TOC file
diff --git a/src/bin/pg_dump/pg_backup_null.c b/src/bin/pg_dump/pg_backup_null.c
index bf1e6e6..1e86aaa 100644
--- a/src/bin/pg_dump/pg_backup_null.c
+++ b/src/bin/pg_dump/pg_backup_null.c
@@ -68,8 +68,6 @@ InitArchiveFmt_Null(ArchiveHandle *AH)
/* Initialize LO buffering */
AH->lo_buf_size = LOBBUFSIZE;
AH->lo_buf = (void *) malloc(LOBBUFSIZE);
- if (AH->lo_buf == NULL)
- die_horribly(AH, NULL, "out of memory\n");
/*
* Now prevent reading...
diff --git a/src/bin/pg_dump/pg_backup_tar.c b/src/bin/pg_dump/pg_backup_tar.c
index 041f9f9..3c9e01d 100644
--- a/src/bin/pg_dump/pg_backup_tar.c
+++ b/src/bin/pg_dump/pg_backup_tar.c
@@ -167,8 +167,6 @@ InitArchiveFmt_Tar(ArchiveHandle *AH)
/* Initialize LO buffering */
AH->lo_buf_size = LOBBUFSIZE;
AH->lo_buf = (void *) malloc(LOBBUFSIZE);
- if (AH->lo_buf == NULL)
- die_horribly(AH, modulename, "out of memory\n");
/*
* Now open the tar file, and load the TOC if we're in read mode.
@@ -1011,8 +1009,6 @@ tarPrintf(ArchiveHandle *AH, TAR_MEMBER *th, const char *fmt,...)
free(p);
bSize *= 2;
p = (char *) malloc(bSize);
- if (p == NULL)
- die_horribly(AH, modulename, "out of memory\n");
va_start(ap, fmt);
cnt = vsnprintf(p, bSize, fmt, ap);
va_end(ap);
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index c95614b..000e8e3 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -520,10 +520,23 @@ extern void simple_string_list_append(SimpleStringList *list, const char *val);
extern bool simple_oid_list_member(SimpleOidList *list, Oid val);
extern bool simple_string_list_member(SimpleStringList *list, const char *val);
+/*
+ * Redefinitions of the memory allocation functions that call
+ * die_horribly() when they fail instead of returning NULL.
+ */
extern char *pg_strdup(const char *string);
extern void *pg_malloc(size_t size);
extern void *pg_calloc(size_t nmemb, size_t size);
extern void *pg_realloc(void *ptr, size_t size);
+#ifndef PG_DUMP_NO_ALLOC_REDEFINE
+#ifdef strdup
+#undef strdup
+#endif
+#define strdup(x) pg_strdup(x)
+#define malloc(x) pg_malloc(x)
+#define calloc(x,y) pg_calloc(x, y)
+#define realloc(x,y) pg_realloc(x, y)
+#endif
extern void check_conn_and_db(void);
extern void exit_nicely(void);
diff --git a/src/bin/pg_dump/pg_dump_alloc.c b/src/bin/pg_dump/pg_dump_alloc.c
new file mode 100644
index 0000000..7b252a7
--- /dev/null
+++ b/src/bin/pg_dump/pg_dump_alloc.c
@@ -0,0 +1,54 @@
+#define PG_DUMP_NO_ALLOC_REDEFINE
+#include "pg_backup.h"
+
+/*
+ * Safer versions of some standard C library functions. If an
+ * out-of-memory condition occurs, these functions will bail out
+ * safely; therefore, their return value is guaranteed to be non-NULL.
+ */
+
+char *
+pg_strdup(const char *string)
+{
+ char *tmp;
+
+ if (!string)
+ exit_horribly(NULL, NULL, "cannot duplicate null pointer\n");
+ tmp = strdup(string);
+ if (!tmp)
+ exit_horribly(NULL, NULL, "out of memory\n");
+ return tmp;
+}
+
+void *
+pg_malloc(size_t size)
+{
+ void *tmp;
+
+ tmp = malloc(size);
+ if (!tmp)
+ exit_horribly(NULL, NULL, "out of memory\n");
+ return tmp;
+}
+
+void *
+pg_calloc(size_t nmemb, size_t size)
+{
+ void *tmp;
+
+ tmp = calloc(nmemb, size);
+ if (!tmp)
+ exit_horribly(NULL, NULL, "out of memory\n");
+ return tmp;
+}
+
+void *
+pg_realloc(void *ptr, size_t size)
+{
+ void *tmp;
+
+ tmp = realloc(ptr, size);
+ if (!tmp)
+ exit_horribly(NULL, NULL, "out of memory\n");
+ return tmp;
+}
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index 963f734..09acb51 100644
--- a/src/bin/pg_dump/pg_dump_sort.c
+++ b/src/bin/pg_dump/pg_dump_sort.c
@@ -228,8 +228,6 @@ sortDumpableObjects(DumpableObject **objs, int numObjs)
return;
ordering = (DumpableObject **) malloc(numObjs * sizeof(DumpableObject *));
- if (ordering == NULL)
- exit_horribly(NULL, modulename, "out of memory\n");
while (!TopoSort(objs, numObjs, ordering, &nOrdering))
findDependencyLoops(ordering, nOrdering, numObjs);
@@ -302,8 +300,6 @@ TopoSort(DumpableObject **objs,
/* Create workspace for the above-described heap */
pendingHeap = (int *) malloc(numObjs * sizeof(int));
- if (pendingHeap == NULL)
- exit_horribly(NULL, modulename, "out of memory\n");
/*
* Scan the constraints, and for each item in the input, generate a count
@@ -313,12 +309,8 @@ TopoSort(DumpableObject **objs,
* dumpId j.
*/
beforeConstraints = (int *) malloc((maxDumpId + 1) * sizeof(int));
- if (beforeConstraints == NULL)
- exit_horribly(NULL, modulename, "out of memory\n");
memset(beforeConstraints, 0, (maxDumpId + 1) * sizeof(int));
idMap = (int *) malloc((maxDumpId + 1) * sizeof(int));
- if (idMap == NULL)
- exit_horribly(NULL, modulename, "out of memory\n");
for (i = 0; i < numObjs; i++)
{
obj = objs[i];
@@ -517,8 +509,6 @@ findDependencyLoops(DumpableObject **objs, int nObjs, int totObjs)
int i;
workspace = (DumpableObject **) malloc(totObjs * sizeof(DumpableObject *));
- if (workspace == NULL)
- exit_horribly(NULL, modulename, "out of memory\n");
initiallen = 0;
fixedloop = false;
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index b3ad2ea..a47bf63 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -1646,12 +1646,6 @@ connectDatabase(const char *dbname, const char *pghost, const char *pgport,
const char **keywords = malloc(PARAMS_ARRAY_SIZE * sizeof(*keywords));
const char **values = malloc(PARAMS_ARRAY_SIZE * sizeof(*values));
- if (!keywords || !values)
- {
- fprintf(stderr, _("%s: out of memory\n"), progname);
- exit(1);
- }
-
keywords[0] = "host";
values[0] = pghost;
keywords[1] = "port";
@@ -1866,3 +1860,21 @@ doShellQuoting(PQExpBuffer buf, const char *str)
appendPQExpBufferChar(buf, '"');
#endif /* WIN32 */
}
+
+/*
+ * exit_horribly() is called from the redefined memory allocation
+ * routines if we run out of memory.
+ */
+void
+exit_horribly(Archive *AH, const char *modulename, const char *fmt,...)
+{
+ va_list ap;
+
+ fprintf(stderr, "%s: ", progname);
+
+ va_start(ap, fmt);
+ vfprintf(stderr, _(fmt), ap);
+ va_end(ap);
+
+ exit(1);
+}
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
