Re: [COMMITTERS] pgsql: Add PF_PRINTF_ATTRIBUTE to on_exit_msg_fmt.

On 26.03.2013 09:51, Heikki Linnakangas wrote:
> On 26.03.2013 02:02, Tom Lane wrote:
>> Heikki Linnakangas<hlinnakangas(at)vmware(dot)com> writes:
>>> On 25.03.2013 15:36, Tom Lane wrote:
>>>> Heikki Linnakangas<heikki(dot)linnakangas(at)iki(dot)fi> writes:
>>>>> Add PF_PRINTF_ATTRIBUTE to on_exit_msg_fmt.
>>>>> Per warning from -Wmissing-format-attribute.
>>
>>>> Hm, this is exactly what I removed yesterday, because it makes the
>>>> build
>>>> fail outright on old gcc:
>>
>>> The attached seems to work. With this patch, on_exit_msg_func() is gone.
>>> There's a different implementation of exit_horribly for pg_dumpall and
>>> pg_dump/restore. In pg_dumpall, it just calls vwrite_msg(). In
>>> pg_dump/restore's version, the logic from parallel_exit_msg_func() is
>>> moved directly to exit_horribly().
>>
>> Seems probably reasonable, though if we're taking exit_horribly out of
>> dumputils.c, meseems it ought not be declared in dumputils.h anymore.
>> Can we put that declaration someplace else, rather than commenting it
>> with an apology?
>
> Ugh, the patch I posted doesn't actually work, because dumputils.c is
> also used in psql and some scripts, so you get a linker error in those.
> psql and scripts don't use exit_horribly or many of the other functions
> in dumputils.c, so I think we should split dumputils.c into two parts
> anyway. fmtId and the other functions that are used by psql in one file,
> and the functions that are only shared between pg_dumpall and pg_dump in
> another. Then there's also functions that are used by pg_dump and
> pg_restore, but not pg_dumpall or psql.
>
> I'll try moving things around a bit...

This is what I came up with. I created a new file, misc.c (for lack of a
better name), for things that are shared by pg_dump and pg_restore, but
not pg_dumpall or other programs. I moved all the parallel stuff from
dumputils.c to parallel.c, and everything else that's not used outside
pg_dump and pg_restore to misc.c. I moved exit_horribly() to parallel.c,
because it needs to do things differently in parallel mode.

I still used a function pointer, not for the printf-style message
printing routine, but for making dumputils.c independent of parallel
mode. getThreadLocalPQBuffer() is now a function pointer; the default
implementation just uses a static variable, but when pg_dump/restore
enters parallel mode, it points the function pointer to a version that
uses thread-local storage (on windows).

- Heikki