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

From: Craig Ringer <craig(at)postnewspapers(dot)com(dot)au>
To: Magnus Hagander <magnus(at)hagander(dot)net>
Cc: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, Dave Page <dpage(at)pgadmin(dot)org>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Re: Proposed Windows-specific change: Enable crash dumps (like core files)
Date: 2010-12-14 06:02:27
Message-ID: 4D070873.7070000@postnewspapers.com.au
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I've attached an updated patch that fixes a failure when compiling on
gcc/linux. The no-op inline installCrashDumpHandler() for unsupported
platforms was not declared static, so it was not being optimized out of
objects it wasn't used in and was causing symbol collisions during linkage.

"make check" now passes correctly on Linux and no warnings are generated
during compilation.
=======================
All 124 tests passed.
=======================

Sorry about this mess-up; I don't recall having to use "inline static"
when using inlines in headers with c++, but as that was a while ago (and
C++) my recollection can't be trusted anyway.

Your thoughts on the following questions from before would be much
appreciated:

> 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 .

Attachment Content-Type Size
0001-Add-support-for-generating-a-crash-dump-on-backend-p.patch text/plain 0 bytes

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2010-12-14 06:12:30 Re: hstores in pl/python
Previous Message Christophe Pettus 2010-12-14 05:32:29 Re: hstores in pl/python