Allow pg_dumpall to use dumpmem.c functions, simplify exit code

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  <bruce@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');