On Tue, Sep 27, 2022 at 1:54 PM Kyotaro Horiguchi <horikyota....@gmail.com> wrote: > > This commit introduced BackupState struct. The comment of > do_pg_backup_start says that: > > > * It fills in backup_state with the information required for the backup, > > And the parameters are: > > > do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces, > > BackupState *state, StringInfo > > tblspcmapfile) > > So backup_state is different from both the type BackupState and the > parameter state. I find it annoying. Don't we either rename the > parameter or fix the comment? > > The parameter "state" sounds a bit too generic. So I prefer to rename > the parameter to backup_state, as the attached. > > What do you think about this?
-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 -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
