> On Feb 5, 2026, at 09:54, Yugo Nagata <[email protected]> wrote:
>
> On Wed, 4 Feb 2026 17:20:49 +0800
> Chao Li <[email protected]> wrote:
>
>> Hi Hackers,
>>
>> This comes from a previous review and has been on my to-do list for a while.
>>
>> Since src/bin/pg_upgrade/slru_io.c includes postgres_fe.h, it is frontend
>> code, so backend memory contexts are not used here.
>>
>> In the current code:
>> ```
>> void
>> FreeSlruWrite(SlruSegState *state)
>> {
>> Assert(state->writing);
>>
>> SlruFlush(state);
>>
>> if (state->fd != -1)
>> close(state->fd);
>> pg_free(state);
>> }
>> ```
>>
>> the SlruSegState itself is freed, but state->dir and state->fn are not,
>> which results in a memory leak during pg_upgrade runs. More generally, I
>> don’t see a reason to free an object itself without also freeing the memory
>> owned by its members.
>
> As far as I know, AllocSlruSegState() and FreeSlruWrite() are not called
> repeatedly in a loop, and the amount of memory involved is small, so the
> impact of the leak seems limited. That said, I agree that it is better to
> also free the memory owned by the struct members.
>
> Regards,
> Yugo Nagata
>
Hi Yugo-san,
Thank you very much for the review. Yes, the leak itself is tiny, the main
point for me is that the memory owned by the struct members is not freed, which
can be a bit confusing for code readers. As for the change from pstrdup() to
pg_strdup(), that felt too trivial to justify a separate patch, so I took this
as an opportunity to clean it up at the same time.
BTW, this is the CF entry: https://commitfest.postgresql.org/patch/6462/, you
may mark yourself as a reviewer.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
