> On Feb 5, 2026, at 09:54, Yugo Nagata <nagata@sraoss.co.jp> wrote:
>
> On Wed, 4 Feb 2026 17:20:49 +0800
> Chao Li <li.evan.chao@gmail.com> 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
usedhere.
>>
>> 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_upgraderuns. More generally, I don’t see a reason to free an object itself without also freeing the memory owned by
itsmembers.
>
> 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
structmembers 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
sametime.
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/
