On Fri, Sep 19, 2025 at 8:21 AM Chao Li <li.evan.chao@gmail.com> wrote:
>
>
>
> On Sep 19, 2025, at 00:48, Fujii Masao <masao.fujii@gmail.com> wrote:
>
> Fujii Masao
> <v2-0001-Make-invalid-primary_slot_name-follow-standard-GU.patch>
>
>
> ```
> +error:
> + if (elevel == 0)
> + {
> + GUC_check_errdetail("%s", err_msg);
> + if (err_hint != NULL)
> + GUC_check_errhint("%s", err_hint);
> + }
> + else
> + ereport(elevel,
> + (errcode(err_code),
> + errmsg("%s", err_msg),
> + (err_hint != NULL) ? errhint("%s", err_hint) : 0));
> +
> + return false;
> ```
>
> Do we need to free memory pointed by err_msg and err_hint that were allocated from psprintf()? The code comment of
psprintf()says caller is responsible for free the memory.
Thanks for the review!
I had thought that since the short-lived "config file processing" memory context
is used when processing the configuration file and then freed afterward,
there was no need for ReplicationSlotValidateName() to call pfree().
However, when it's called by StartupReorderBuffer() with elevel = DEBUG2,
allocations seem to be made in TopMemoryContext, so they could persist
much longer.
So I agree it's safer to free them explicitly. In the attached updated patch,
ReplicationSlotValidateName() now pfrees err_msg and err_hint when needed.
Regards,
--
Fujii Masao
