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