Re: Re: Proposed Windows-specific change: Enable crash dumps (like core files)

On 6/12/2010 12:57 PM, Craig Ringer wrote:
> On 22/11/2010 7:37 PM, Magnus Hagander wrote:
>
>> Finally getting to looking at this one - sorry about the very long delay.
>
> Ditto, I'm afraid.

Oh, I forgot to mention in the patch email: I'm not sure I've taken the
right approach in terms of how I've hooked the crash dump code into the
build system. I'm fairly happy with when it's actually inited, and with
the win32 portfile (ports/win32/crashdump.c) - what I'm not so sure
about is how I've handled the conditional compilation by platform.

Currently, src/backend/ports/win32/crashdump.c is compiled on win32. It
exports a generic win32-garbage-free function:

void
installCrashDumpHandler();

I was just going to create a no-op version of the same function in a
(new) file src/backend/ports/crashdump.c , but I was having problems
figuring out how to sensibly express "compile this file for everything
except these ports". In addition, doing it this way means there's still
a pointless no-op function call in each backend start and some useless
symbols - maybe not a big worry, but not ideal.

What I ended up doing was putting a conditionally compiled switch in
port.h that produces an inline no-op version of installCrashDumpHandler
for unsupported platforms, and an extern for supported platforms
(currently win32). That'll optimize out completely on unsupported
platforms, which is nice if unimportant.

I'm just not sure if port.h is really the right place and if it's only
used by the backend. I considered a new header included by postgres.h
(since it's really backend-only stuff) but as it seems Pg generally
prefers bigger headers over more little headers, I popped it in port.h .

Comments?

--
Craig Ringer