Bruce Momjian wrote:
> > I was wondering if it wouldn't make more sense to have pg_dumpall supply
> > its own version of exit_horribly to avoid separate pg_malloc and
> > pg_strdup ... but then those routines are so tiny that it hardly makes a
> > difference.
> >
> > Another thing I wondered when seeing the original commit is the fact
> > that the old code passed the AH to exit_horribly in some places, whereas
> > the new one simply uses NULL.
...
>
> I am thinking we should just get rid of the whole AH passing.
>
> I have always felt the pg_dump code is overly complex, and this is
> confirming my suspicion.
I have developed the attached patch which accomplishes this. I was also
able to move the write_msg function into dumputils.c (which is linked to
pg_dumpall), which allows pg_dumpall to use the new dumpmem.c functions,
and I removed its private ones.
FYI, I found write_msg() was a useless valist trampoline so I removed
the trampoline code and renamed _write_msg() to write_msg(). I also
modified the MSVC code.
--
Bruce Momjian <br...@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
diff --git a/src/bin/pg_dump/Makefile b/src/bin/pg_dump/Makefile
new file mode 100644
index 4e8e421..09101d5
*** a/src/bin/pg_dump/Makefile
--- b/src/bin/pg_dump/Makefile
*************** pg_dump: pg_dump.o common.o pg_dump_sort
*** 35,42 ****
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)
install: all installdirs
$(INSTALL_PROGRAM) pg_dump$(X) '$(DESTDIR)$(bindir)'/pg_dump$(X)
--- 35,42 ----
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 dumpmem.o $(KEYWRDOBJS) | submake-libpq submake-libpgport
! $(CC) $(CFLAGS) pg_dumpall.o dumputils.o dumpmem.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/dumpmem.c b/src/bin/pg_dump/dumpmem.c
new file mode 100644
index a50f4f5..a71e217
*** a/src/bin/pg_dump/dumpmem.c
--- b/src/bin/pg_dump/dumpmem.c
***************
*** 14,20 ****
*-------------------------------------------------------------------------
*/
#include "postgres_fe.h"
! #include "pg_backup.h"
#include "dumpmem.h"
#include <ctype.h>
--- 14,20 ----
*-------------------------------------------------------------------------
*/
#include "postgres_fe.h"
! #include "dumputils.h"
#include "dumpmem.h"
#include <ctype.h>
*************** pg_strdup(const char *string)
*** 32,41 ****
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;
}
--- 32,41 ----
char *tmp;
if (!string)
! exit_horribly(NULL, "cannot duplicate null pointer\n");
tmp = strdup(string);
if (!tmp)
! exit_horribly(NULL, "out of memory\n");
return tmp;
}
*************** pg_malloc(size_t size)
*** 46,52 ****
tmp = malloc(size);
if (!tmp)
! exit_horribly(NULL, NULL, "out of memory\n");
return tmp;
}
--- 46,52 ----
tmp = malloc(size);
if (!tmp)
! exit_horribly(NULL, "out of memory\n");
return tmp;
}
*************** pg_calloc(size_t nmemb, size_t size)
*** 57,63 ****
tmp = calloc(nmemb, size);
if (!tmp)
! exit_horribly(NULL, NULL, _("out of memory\n"));
return tmp;
}
--- 57,63 ----
tmp = calloc(nmemb, size);
if (!tmp)
! exit_horribly(NULL, _("out of memory\n"));
return tmp;
}
*************** pg_realloc(void *ptr, size_t size)
*** 68,73 ****
tmp = realloc(ptr, size);
if (!tmp)
! exit_horribly(NULL, NULL, _("out of memory\n"));
return tmp;
}
--- 68,73 ----
tmp = realloc(ptr, size);
if (!tmp)
! exit_horribly(NULL, _("out of memory\n"));
return tmp;
}
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
new file mode 100644
index 92b9d28..39601e6
*** a/src/bin/pg_dump/dumputils.c
--- b/src/bin/pg_dump/dumputils.c
***************
*** 23,28 ****
--- 23,29 ----
int quote_all_identifiers = 0;
+ const char *progname;
#define supports_grant_options(version) ((version) >= 70400)
*************** emitShSecLabels(PGconn *conn, PGresult *
*** 1211,1213 ****
--- 1212,1244 ----
appendPQExpBuffer(buffer, ";\n");
}
}
+
+
+ void
+ write_msg(const char *modulename, const char *fmt,...)
+ {
+ va_list ap;
+
+ va_start(ap, fmt);
+ if (modulename)
+ fprintf(stderr, "%s: [%s] ", progname, _(modulename));
+ else
+ fprintf(stderr, "%s: ", progname);
+ vfprintf(stderr, _(fmt), ap);
+ va_end(ap);
+ }
+
+
+ void
+ exit_horribly(const char *modulename, const char *fmt,...)
+ {
+ va_list ap;
+
+ va_start(ap, fmt);
+ write_msg(modulename, fmt, ap);
+ va_end(ap);
+
+ exit(1);
+ }
+
+
diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h
new file mode 100644
index 40bbc81..62d8080
*** a/src/bin/pg_dump/dumputils.h
--- b/src/bin/pg_dump/dumputils.h
*************** extern void buildShSecLabelQuery(PGconn
*** 51,55 ****
--- 51,59 ----
uint32 objectId, PQExpBuffer sql);
extern void emitShSecLabels(PGconn *conn, PGresult *res,
PQExpBuffer buffer, const char *target, const char *objname);
+ extern void write_msg(const char *modulename, const char *fmt,...)
+ __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 3)));
+ extern void exit_horribly(const char *modulename, const char *fmt,...)
+ __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 3)));
#endif /* DUMPUTILS_H */
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
new file mode 100644
index ce12a41..8168cff
*** a/src/bin/pg_dump/pg_backup.h
--- b/src/bin/pg_dump/pg_backup.h
*************** typedef struct _restoreOptions
*** 150,159 ****
* Main archiver interface.
*/
- extern void
- exit_horribly(Archive *AH, const char *modulename, const char *fmt,...)
- __attribute__((format(PG_PRINTF_ATTRIBUTE, 3, 4)));
-
/* Lets the archive know we have a DB connection to shutdown if it dies */
--- 150,155 ----
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
new file mode 100644
index 164d593..1eb2b8b
*** a/src/bin/pg_dump/pg_backup_archiver.c
--- b/src/bin/pg_dump/pg_backup_archiver.c
*************** typedef struct _outputContext
*** 84,91 ****
int gzOut;
} OutputContext;
- const char *progname;
-
static const char *modulename = gettext_noop("archiver");
/* index array created by fix_dependencies -- only used in parallel restore */
--- 84,89 ----
*************** static int _discoverArchiveFormat(Archiv
*** 120,126 ****
static int RestoringToDB(ArchiveHandle *AH);
static void dump_lo_buf(ArchiveHandle *AH);
- static void _write_msg(const char *modulename, const char *fmt, va_list ap) __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 0)));
static void _die_horribly(ArchiveHandle *AH, const char *modulename, const char *fmt, va_list ap) __attribute__((format(PG_PRINTF_ATTRIBUTE, 3, 0)));
static void dumpTimestamp(ArchiveHandle *AH, const char *msg, time_t tim);
--- 118,123 ----
*************** ahlog(ArchiveHandle *AH, int level, cons
*** 1302,1308 ****
return;
va_start(ap, fmt);
! _write_msg(NULL, fmt, ap);
va_end(ap);
}
--- 1299,1305 ----
return;
va_start(ap, fmt);
! write_msg(NULL, fmt, ap);
va_end(ap);
}
*************** ahwrite(const void *ptr, size_t size, si
*** 1420,1451 ****
}
}
- /* Common exit code */
- static void
- _write_msg(const char *modulename, const char *fmt, va_list ap)
- {
- if (modulename)
- fprintf(stderr, "%s: [%s] ", progname, _(modulename));
- else
- fprintf(stderr, "%s: ", progname);
- vfprintf(stderr, _(fmt), ap);
- }
-
- void
- write_msg(const char *modulename, const char *fmt,...)
- {
- va_list ap;
-
- va_start(ap, fmt);
- _write_msg(modulename, fmt, ap);
- va_end(ap);
- }
-
static void
_die_horribly(ArchiveHandle *AH, const char *modulename, const char *fmt, va_list ap)
{
! _write_msg(modulename, fmt, ap);
if (AH)
{
--- 1417,1427 ----
}
}
static void
_die_horribly(ArchiveHandle *AH, const char *modulename, const char *fmt, va_list ap)
{
! write_msg(modulename, fmt, ap);
if (AH)
{
*************** _die_horribly(ArchiveHandle *AH, const c
*** 1458,1474 ****
exit(1);
}
- /* External use */
- void
- exit_horribly(Archive *AH, const char *modulename, const char *fmt,...)
- {
- va_list ap;
-
- va_start(ap, fmt);
- _die_horribly((ArchiveHandle *) AH, modulename, fmt, ap);
- va_end(ap);
- }
-
/* Archiver use (just different arg declaration) */
void
die_horribly(ArchiveHandle *AH, const char *modulename, const char *fmt,...)
--- 1434,1439 ----
*************** warn_or_die_horribly(ArchiveHandle *AH,
*** 1524,1530 ****
_die_horribly(AH, modulename, fmt, ap);
else
{
! _write_msg(modulename, fmt, ap);
AH->public.n_errors++;
}
va_end(ap);
--- 1489,1495 ----
_die_horribly(AH, modulename, fmt, ap);
else
{
! write_msg(modulename, fmt, ap);
AH->public.n_errors++;
}
va_end(ap);
diff --git a/src/bin/pg_dump/pg_backup_archiver.h b/src/bin/pg_dump/pg_backup_archiver.h
new file mode 100644
index 8a3a6f9..d1e202f
*** a/src/bin/pg_dump/pg_backup_archiver.h
--- b/src/bin/pg_dump/pg_backup_archiver.h
*************** extern const char *progname;
*** 303,309 ****
extern void die_horribly(ArchiveHandle *AH, const char *modulename, const char *fmt,...) __attribute__((format(PG_PRINTF_ATTRIBUTE, 3, 4)));
extern void warn_or_die_horribly(ArchiveHandle *AH, const char *modulename, const char *fmt,...) __attribute__((format(PG_PRINTF_ATTRIBUTE, 3, 4)));
- extern void write_msg(const char *modulename, const char *fmt,...) __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 3)));
extern void WriteTOC(ArchiveHandle *AH);
extern void ReadTOC(ArchiveHandle *AH);
--- 303,308 ----
diff --git a/src/bin/pg_dump/pg_backup_custom.c b/src/bin/pg_dump/pg_backup_custom.c
new file mode 100644
index b2f3196..31fa373
*** a/src/bin/pg_dump/pg_backup_custom.c
--- b/src/bin/pg_dump/pg_backup_custom.c
***************
*** 25,30 ****
--- 25,31 ----
*/
#include "compress_io.h"
+ #include "dumputils.h"
#include "dumpmem.h"
/*--------
diff --git a/src/bin/pg_dump/pg_backup_files.c b/src/bin/pg_dump/pg_backup_files.c
new file mode 100644
index 85373b5..ffcbb8f
*** a/src/bin/pg_dump/pg_backup_files.c
--- b/src/bin/pg_dump/pg_backup_files.c
***************
*** 26,31 ****
--- 26,32 ----
*/
#include "pg_backup_archiver.h"
+ #include "dumputils.h"
#include "dumpmem.h"
static void _ArchiveEntry(ArchiveHandle *AH, TocEntry *te);
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
new file mode 100644
index 8023450..368208d
*** a/src/bin/pg_dump/pg_dump_sort.c
--- b/src/bin/pg_dump/pg_dump_sort.c
***************
*** 14,19 ****
--- 14,20 ----
*-------------------------------------------------------------------------
*/
#include "pg_backup_archiver.h"
+ #include "dumputils.h"
#include "dumpmem.h"
static const char *modulename = gettext_noop("sorter");
*************** TopoSort(DumpableObject **objs,
*** 315,327 ****
obj = objs[i];
j = obj->dumpId;
if (j <= 0 || j > maxDumpId)
! exit_horribly(NULL, modulename, "invalid dumpId %d\n", j);
idMap[j] = i;
for (j = 0; j < obj->nDeps; j++)
{
k = obj->dependencies[j];
if (k <= 0 || k > maxDumpId)
! exit_horribly(NULL, modulename, "invalid dependency %d\n", k);
beforeConstraints[k]++;
}
}
--- 316,328 ----
obj = objs[i];
j = obj->dumpId;
if (j <= 0 || j > maxDumpId)
! exit_horribly(modulename, "invalid dumpId %d\n", j);
idMap[j] = i;
for (j = 0; j < obj->nDeps; j++)
{
k = obj->dependencies[j];
if (k <= 0 || k > maxDumpId)
! exit_horribly(modulename, "invalid dependency %d\n", k);
beforeConstraints[k]++;
}
}
*************** findDependencyLoops(DumpableObject **obj
*** 541,547 ****
/* We'd better have fixed at least one loop */
if (!fixedloop)
! exit_horribly(NULL, modulename, "could not identify dependency loop\n");
free(workspace);
}
--- 542,548 ----
/* We'd better have fixed at least one loop */
if (!fixedloop)
! exit_horribly(modulename, "could not identify dependency loop\n");
free(workspace);
}
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
new file mode 100644
index 4782e68..4833b22
*** a/src/bin/pg_dump/pg_dumpall.c
--- b/src/bin/pg_dump/pg_dumpall.c
***************
*** 23,28 ****
--- 23,29 ----
#include "getopt_long.h"
#include "dumputils.h"
+ #include "dumpmem.h"
#include "pg_backup.h"
/* version string we expect back from pg_dump */
*************** doShellQuoting(PQExpBuffer buf, const ch
*** 1908,1948 ****
appendPQExpBufferChar(buf, '"');
#endif /* WIN32 */
}
-
-
- /*
- * Simpler versions of common.c functions.
- */
-
- char *
- pg_strdup(const char *string)
- {
- char *tmp;
-
- if (!string)
- {
- fprintf(stderr, "cannot duplicate null pointer\n");
- exit(1);
- }
- tmp = strdup(string);
- if (!tmp)
- {
- fprintf(stderr, _("%s: out of memory\n"), progname);
- exit(1);
- }
- return tmp;
- }
-
- void *
- pg_malloc(size_t size)
- {
- void *tmp;
-
- tmp = malloc(size);
- if (!tmp)
- {
- fprintf(stderr, _("%s: out of memory\n"), progname);
- exit(1);
- }
- return tmp;
- }
--- 1909,1911 ----
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
new file mode 100644
index 94ecb65..fb83224
*** a/src/tools/msvc/Mkvcbuild.pm
--- b/src/tools/msvc/Mkvcbuild.pm
*************** sub mkvcbuild
*** 355,360 ****
--- 355,361 ----
$pgdumpall->AddIncludeDir('src\backend');
$pgdumpall->AddFile('src\bin\pg_dump\pg_dumpall.c');
$pgdumpall->AddFile('src\bin\pg_dump\dumputils.c');
+ $pgdumpall->AddFile('src\bin\pg_dump\dumpmem.c');
$pgdumpall->AddFile('src\bin\pg_dump\keywords.c');
$pgdumpall->AddFile('src\backend\parser\kwlookup.c');
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
