Re: pgsql: Make pg_dumpall build with the right object files under MSVC.

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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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