Michael Paquier <michael@paquier.xyz> writes:
> On Wed, Mar 13, 2024 at 02:33:52PM +0100, Peter Eisentraut wrote:
>> The 0002 patch looks sensible. It would be good to fix that, otherwise it
>> could have some confusing outcomes in the future.
>
> You mean if we begin to use %m in future callers of
> emit_tap_output_v(), hypothetically? That's a fair argument.
Yeah, developers would rightfully expect to be able to use %m with
anything that takes a printf format string. Case in point: when I was
first doing the conversion I did change the bail() and diag() calls in
pg_regress to %m, and only while I was preparing the patch for
submission did I think to check the actual implementation to see if it
was safe to do so.
The alternative would be to document that you can't use %m with these
functions, which is silly IMO, given how simple the fix is.
One minor improvement I can think of is to add a comment by the
save_errno declaration noting that it's needed in order to support %m.
- ilmari
