Re: Refactor backup related code (was: Is it correct to say, "invalid data in file \"%s\"", BACKUP_LABEL_FILE in do_pg_backup_stop?)

On Tue, Sep 27, 2022 at 2:20 PM Kyotaro Horiguchi
<horikyota(dot)ntt(at)gmail(dot)com> wrote:
>
> > -1 from me. We have the function context and the structure name there
> > to represent that the parameter name 'state' is actually 'backup
> > state'. I don't think we gain anything here. Whenever the BackupState
> > is used away from any function, the variable name backup_state is
> > used, static variable in xlogfuncs.c
>
> There's no shadowing caused by the change. If we mind the same
> variable names between files, we could rename backup_state in
> xlogfuncs.c to process_backup_state or session_backup_state.

-1.

> If this is still unacceptable, I propose to change the comment. (I
> found that the previous patch forgets about do_pg_backup_stop())
>
> - * It fills in backup_state with the information required for the backup,
> + * It fills in the parameter "state" with the information required for the backup,

+1. There's another place that uses backup_state in the comments. I
modified that as well. Please see the attached patch.

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com