Re: pgsql: Make pg_dumpall build with the right object files under MSVC. - Mailing list pgsql-committers

Alvaro Herrera wrote:
>
> Excerpts from Andrew Dunstan's message of lun nov 28 14:40:24 -0300 2011:
> >
> > On 11/28/2011 11:33 AM, Bruce Momjian wrote:
>
> > > In summary, for those watching, pg_dump and pg_restore used to share
> > > OBJS, and with my new patch, dumpmem.c is now shared by those and
> > > pg_dumpall.  Seems the MSVC code previously could not handle that case,
> > > which is fixed by this patch.
> >
> > Er, no. Only dumputils.c is shared with pg_dumpall. dumpmem.c is not
> > (see the Makefile).
> >
> > The problem that arose is that pg_dumpall has its own (non-static)
> > versions of pg_malloc and pg_strdup, so we got duplicate symbol errors
> > from the newly declared dumpmem.c functions when we erroneously tried
> > linking it in on MSVC.
>
> 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.

Good point.  Our old 9.1 code had:

    common.c:               exit_horribly(NULL, NULL, "cannot duplicate null pointer\n");
    common.c:               exit_horribly(NULL, NULL, "out of memory\n");
    common.c:               exit_horribly(NULL, NULL, "out of memory\n");
    common.c:               exit_horribly(NULL, NULL, "out of memory\n");
    common.c:               exit_horribly(NULL, NULL, "out of memory\n");
-->    pg_backup_archiver.c:                   exit_horribly(AH, modulename, "out of memory\n");
    pg_backup_archiver.c:exit_horribly(Archive *AH, const char *modulename, const char *fmt,...)
    pg_dump_sort.c:         exit_horribly(NULL, modulename, "out of memory\n");
    pg_dump_sort.c:         exit_horribly(NULL, modulename, "out of memory\n");
    pg_dump_sort.c:         exit_horribly(NULL, modulename, "out of memory\n");
    pg_dump_sort.c:         exit_horribly(NULL, modulename, "out of memory\n");
    pg_dump_sort.c:                 exit_horribly(NULL, modulename, "invalid dumpId %d\n", j);
    pg_dump_sort.c:                         exit_horribly(NULL, modulename, "invalid dependency %d\n", k);
    pg_dump_sort.c:         exit_horribly(NULL, modulename, "out of memory\n");
    pg_dump_sort.c:         exit_horribly(NULL, modulename, "could not identify dependency loop\n");

while our new code has:

    dumpmem.c:              exit_horribly(NULL, NULL, "cannot duplicate null pointer\n");
    dumpmem.c:              exit_horribly(NULL, NULL, "out of memory\n");
    dumpmem.c:              exit_horribly(NULL, NULL, "out of memory\n");
    dumpmem.c:              exit_horribly(NULL, NULL, _("out of memory\n"));
    dumpmem.c:              exit_horribly(NULL, NULL, _("out of memory\n"));
    pg_backup_archiver.c:exit_horribly(Archive *AH, const char *modulename, const char *fmt,...)
    pg_dump_sort.c:                 exit_horribly(NULL, modulename, "invalid dumpId %d\n", j);
    pg_dump_sort.c:                         exit_horribly(NULL, modulename, "invalid dependency %d\n", k);
    pg_dump_sort.c:         exit_horribly(NULL, modulename, "could not identify dependency loop\n");

There is actually one case in the old code where we passed AH, and that
AH is only used for:

    if (AH)
    {
        if (AH->public.verbose)
            write_msg(NULL, "*** aborted because of error\n");
        if (AH->connection)
            PQfinish(AH->connection);
    }

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.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + It's impossible for everything to be true. +