Re: Allowing printf("%m") only where it actually works

Michael Paquier <michael(at)paquier(dot)xyz> writes:
> On Wed, Sep 12, 2018 at 01:40:01PM -0400, Tom Lane wrote:
>> Rebase attached --- no substantive changes.

> - if (handleDLL == NULL)
> - ereport(FATAL,
> - (errmsg_internal("could not load netmsg.dll: error
> - code %lu", GetLastError())));

> In 0001, this is replaced by a non-FATAL error for the backend, which
> does not seem like a good idea to me because the user loses visibility
> with this DDL which canot be loaded. I still have to see this error...

Well, we have to change the code somehow to make it usable in frontend
as well as backend. And we can *not* have it do exit(1) in libpq.
So the solution I chose was to make it act the same as if FormatMessage
were to fail. I don't find this behavior unreasonable: what is really
important is the original error code, not whether we were able to
pretty-print it. I think the ereport(FATAL) coding is a pretty darn
bad idea even in the backend.

> Could you drop the configure checks for snprintf and vsnprintf in a
> separate patch? The cleanup of %m and the removal of those switches
> should be treated separatly in my opinion.

Seems a bit make-worky, but here you go. 0001 is the same as before
(but rebased up to today, so some line numbers change). 0002
changes things so that we always use our snprintf, removing all the
configure logic associated with that. 0003 implements %m in snprintf.c
and adjusts our various printf-wrapper functions to ensure that they
pass errno through reliably. 0004 changes elog.c to rely on %m being
implemented below it.

regards, tom lane