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

On 12/15/2010 01:01 AM, Tom Lane wrote:
> Craig Ringer<craig@postnewspapers.com.au>  writes:
>> 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.
>
> Why in the world would you get involved in that portability mess for a
> function that is called only once?  There's no possible performance
> justification for making it inline.

The main concern I heard voiced when first suggesting this was about 
performance. Given that concern, if I could make it a no-op on 
unix/linux I thought that worth doing.

I'm _much_ happier with a simple, non-ifdef'd extern function 
declaration and compilation of an empty function body on unsupported 
platforms. Given how concerned everyone was about *any* effect on 
backend startup, though, I was concerned that'd be turned down as 
unnecessary bloat.

I've done it a nicer way now, and will post the updated patch once I've 
had a chance to re-test it on my Windows dev box.

> I'm also wondering why you have got conflicting declarations in
> postgres.h and port.h, and why none of these declarations follow
> ANSI C (write "(void)" not "()").

For postgres.h : that's a good question, as I thought I removed that. I 
suspect it was reintroduced when reapplying the patch to my working tree 
to revise it. Whoops.

As for the ansi C style - too much time with C++, though long ago now. I 
think I got the PostgreSQL rules for code formatting right, but missed 
the void param rule.

--
Craig Ringer