On 10/13/25 13:24, Bryan Green wrote:
> On 10/13/25 13:16, Bryan Green wrote:
>> On 10/13/25 13:01, Tom Lane wrote:
>>> Bryan Green <dbryan.green@gmail.com> writes:
>>>> On 10/13/25 10:48, Tom Lane wrote:
>>>>> Huh, that certainly appears broken, but isn't the non-FRONTEND
>>>>> case equally broken? write_stderr() doesn't expect a va_list
>>>>> either. I wonder if this code is ever reached at all.
>>>
>>>> I found two instances of write_stderr(), one in elog.c and another in
>>>> pg_ctl.c.
>>>> They both use functions that correctly handle the va_list-- either
>>>> vsnprintf or vfprintf.
>>>
>>> It's the one in elog.c that the non-FRONTEND case is going to invoke.
>>> But I think you're mistaken that it'll "just work". write_stderr()
>>> is creating its own va_list.
>>>
>>> We probably need to invent vwrite_stderr().
>>>
>>> regards, tom lane
>> Oh, yes, I see it now. I will create the vwrite_stderr()-- probably
>> something like below, and then create a new patch.
>>
>> void
>> vwrite_stderr(const char *fmt, va_list ap)
>> {
>> #ifdef WIN32
>> char errbuf[2048];
>> #endif
>> fmt = _(fmt);
>>
>> #ifndef WIN32
>> vfprintf(stderr, fmt, ap);
>> fflush(stderr);
>> #else
>> vsnprintf(errbuf, sizeof(errbuf), fmt, ap);
>> if (pgwin32_is_service())
>> write_eventlog(ERROR, errbuf, strlen(errbuf));
>> else
>> {
>> write_console(errbuf, strlen(errbuf));
>> fflush(stderr);
>> }
>> #endif
>> }
>>
>> Thanks,
>> Bryan Green
> I mistakenly just hit reply instead of reply all. Apologies for the
> clutter.
Hello,
The problem:
- FRONTEND path: fprintf(stderr, fmt, ap) - incorrect
- non-FRONTEND path: write_stderr(fmt, ap) - also incorrect
The first patch only addressed the fprintf issue. The v2 patch
addresses both.
Both were passing a va_list to variadic functions that expect individual
arguments (...), causing the va_list pointer to be treated as a single
argument rather than properly expanding the arguments.
In order to straighten this out the following items were done:
1. Adding a new vwrite_stderr() function that accepts va_list, following
the standard C library pattern (printf/vprintf, fprintf/vfprintf, etc.)
2. Refactoring write_stderr() to call vwrite_stderr() internally
3. Fixing both paths in log_error():
- FRONTEND: fprintf -> vfprintf
- non-FRONTEND: write_stderr -> vwrite_stderr
This bug likely went unnoticed because these error paths are only hit
when Windows security API calls fail catastrophically (out of
memory,corrupted security subsystem, etc.), which is extremely rare in
practice.
Please review the attached v2 patch.
Thanks,
bg
