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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to