On 23.09.22 04:37, Tom Lane wrote:
> Separately from that: is it really okay to delegate use of a va_list
> argument like that? The other call paths of
> appendPQExpBufferVA[_internal] write va_start/va_end directly around it,
> not somewhere else in the call chain. I'm too tired to language-lawyer
> out what happens when you do it like this, but I'm suspecting that it's
> not well-defined portable behavior.
>
> I think what you probably need to do is export appendPQExpBufferVA
> as-is and require libpq_append_error to provide the error loop.
There was actually a live problem here, maybe not the exact one you had
in mind: When you actually need the "need more space" loop, you must do
va_end() and va_start() before calling down again. Otherwise, the next
va_arg() gets garbage.
It so happens that the error message
"private key file \"%s\" has group or world access; file must have
permissions u=rw (0600) or less if owned by the current user, or
permissions u=rw,g=r (0640) or less if owned by root"
together with an in-tree test location for the file in question just
barely exceeds INITIAL_EXPBUFFER_SIZE (256), and so my previous patch
would fail the "ssl" test suite. Good test coverage. :)
Anyway, I have updated my patch with your suggestion, which should fix
these kinds of issues.