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

On Mon, Sep 24, 2018 at 01:18:47PM -0400, Tom Lane wrote:
> 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.

Ok. I won't fight hard on that. Why changing the error message from
"could not load netmsg.dll" to "unrecognized winsock error" then? The
original error string is much more verbose to grab the context.

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

Thanks for the new versions. The only thing I could find to complain
about is the error message above, the rest looks in nice shape.
--
Michael