Re: C99 compliance for src/port/snprintf.c

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Tue, Aug 14, 2018 at 7:28 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I'm almost tempted to think that the reasons above make this a
>> back-patchable bug fix. Comments?

> No objections to changing the behavior. Have you checked whether
> there are any noticeable performance consequences?

Hard to believe there could be any performance loss. The normal (non
buffer overflowing) code path gets two additional instructions, storing
zero into .nchars and then adding it to the result. There would be more
work added to buffer-overflow cases, but we'd surely more than win that
back by avoiding repeated repalloc-and-try-again cycles.

> Back-patching seems a bit aggressive to me considering that the danger
> is hypothetical. I'd want to have some tangible evidence that
> back-patching was going help somebody. For all we know somebody's got
> an extension which they only use on Windows that happens to be relying
> on the current behavior, although more likely still (IMHO) is that it
> there is little or no code relying on either behavior.

Yeah, it's theoretically possible that there's somebody out there who's
depending on the pre-C99 behavior and never tested their code anywhere
but Windows. But come on, how likely is that really, compared to the
much more plausible risks I already cited? It's not even that easy
to imagine how someone would construct code that had such a dependency
while not being outright buggy in the face of buffer overrun.

BTW, independently of whether to back-patch, it strikes me that what
we ought to do in HEAD (after applying this) is to just assume we have
C99-compliant behavior, and rip out the baroque logic in psnprintf
and appendPQExpBufferVA that tries to deal with pre-C99 snprintf.
I don't expect that'd save any really meaningful number of cycles,
but at least it'd buy back the two added instructions mentioned above.
I suppose we could put in a configure check that verifies whether
the system snprintf returns the right value for overrun cases, though
it's hard to believe there are any platforms that pass the 'z' check
and would fail this one.

regards, tom lane