| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Tianchen Zhang <zhang_tian_chen(at)163(dot)com> |
| Cc: | "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pg_resetwal: Fix wrong directory in log output |
| Date: | 2026-02-03 06:28:22 |
| Message-ID: | 618B6B0A-C762-489E-9E74-939D79369849@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Feb 3, 2026, at 14:16, Tianchen Zhang <zhang_tian_chen(at)163(dot)com> wrote:
>
>
> At 2026-02-03 10:13:53, "Chao Li" <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>>
>>
>>> On Feb 3, 2026, at 09:58, zhang_tian_chen(at)163(dot)com wrote:
>>>
>>> Hi hackers,
>>>
>>> There is a misuse of macro when we output directory information in KillExistingWALSummaries(), pg_resetwal.c. Which should be WALSUMMARYDIR instead of ARCHSTATDIR.
>>>
>>> Best regards,
>>> Tianchen Zhang<v1-0001-Fix-incorrect-directory-macro-in-KillExistingWALS.patch>
>>
>> Indeed a bug. Looking at the code:
>>
>> ```
>> /*
>> * Remove existing WAL summary files
>> */
>> static void
>> KillExistingWALSummaries(void)
>> {
>> DIR *xldir;
>>
>> xldir = opendir(WALSUMMARYDIR);
>> if (xldir == NULL)
>> pg_fatal("could not open directory \"%s\": %m", WALSUMMARYDIR);
>> ...
>>
>> if (errno)
>> pg_fatal("could not read directory \"%s\": %m", WALSUMMARYDIR);
>>
>> if (closedir(xldir))
>> pg_fatal("could not close directory \"%s\": %m", ARCHSTATDIR); <<=== It should really be WALSUMMARYDIR
>> }
>> ```
>>
>> I guess closedir() is hard to fail, that’s why the problem has not been noticed earlier.
>>
>> The patch is straightforward and looks correct.
>>
>> Best regards,
>> --
>> Chao Li (Evan)
>> HighGo Software Co., Ltd.
>> https://www.highgo.com/
>>
>>
>
> As the discussion above, I've updated the patch to v2 that also undefines the function-scoped marco at the end of corresponding fuctions.
>
> Best regards,
> Tianchen Zhang
> <v2-0001-Fix-incorrect-directory-macro-in-KillExistingWALS.patch>
Thanks for updating the patch.
I applied v2 locally and the build passed. I also tried to revert to the wrong macro, now the compiler caught the mistake:
```
pg_resetwal.c:1116:52: error: use of undeclared identifier 'ARCHSTATDIR'
1116 | pg_fatal("could not close directory \"%s\": %m", ARCHSTATDIR);
| ^
1 error generated.
make[3]: *** [pg_resetwal.o] Error 1
```
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Kyotaro Horiguchi | 2026-02-03 06:42:04 | Re: Fix pg_stat_get_backend_wait_event() for aux processes |
| Previous Message | Chao Li | 2026-02-03 06:22:28 | Re: walsender: Assert MyReplicationSlot is set before use |