On Thursday, December 21, 2023 12:25 PM Peter Smith <smithpb2250@gmail.com> wrote:
>
> Here is a minor comment for v51-0001
>
> ======
> src/backend/replication/slot.c
>
> 1.
> +void
> +RereadConfigAndReInitSlotList(List **standby_slots) {
> + char *pre_standby_slot_names;
> +
> + /*
> + * If we are running on a standby, there is no need to reload
> + * standby_slot_names since we do not support syncing slots to
> + cascading
> + * standbys.
> + */
> + if (RecoveryInProgress())
> + {
> + ProcessConfigFile(PGC_SIGHUP);
> + return;
> + }
> +
> + pre_standby_slot_names = pstrdup(standby_slot_names);
> +
> + ProcessConfigFile(PGC_SIGHUP);
> +
> + if (strcmp(pre_standby_slot_names, standby_slot_names) != 0) {
> + list_free(*standby_slots); *standby_slots = GetStandbySlotList(true);
> + }
> +
> + pfree(pre_standby_slot_names);
> +}
>
> Consider below, which seems a simpler way to do that but with just one return
> point and without duplicating the ProcessConfigFile calls:
>
> SUGGESTION
> {
> char *pre_standby_slot_names = pstrdup(standby_slot_names);
>
> ProcessConfigFile(PGC_SIGHUP);
>
> if (!RecoveryInProgress())
> {
> if (strcmp(pre_standby_slot_names, standby_slot_names) != 0)
> {
> list_free(*standby_slots);
> *standby_slots = GetStandbySlotList(true);
> }
> }
>
> pfree(pre_standby_slot_names);
> }
Thanks for the suggestion. I also thought about this, but I'd like to avoid
allocating/freeing memory for the pre_standby_slot_names if not needed.
Best Regards,
Hou zj
