From: | Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: libpq error message refactoring |
Date: | 2022-10-12 07:45:01 |
Message-ID: | 3352b18d-7874-b3ce-3fbe-45dda5777e63@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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.
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2022-10-12 07:46:37 | Re: BufferAlloc: don't take two simultaneous locks |
Previous Message | Michael Paquier | 2022-10-12 07:42:38 | Re: Proposal: allow database-specific role memberships |