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?

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/access/transam/xlog.c 
b/src/backend/access/transam/xlog.c
index 1dd6df0fe1..715d5868eb 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8244,10 +8244,10 @@ issue_xlog_fsync(int fd, XLogSegNo segno, TimeLineID 
tli)
  * function. It creates the necessary starting checkpoint and constructs the
  * backup state and tablespace map.
  *
- * Input parameters are "state" (the backup state), "fast" (if true, we do
- * the checkpoint in immediate mode to make it faster), and "tablespaces"
- * (if non-NULL, indicates a list of tablespaceinfo structs describing the
- * cluster's tablespaces.).
+ * Input parameters are "backup_state", "fast" (if true, we do the checkpoint
+ * in immediate mode to make it faster), and "tablespaces" (if non-NULL,
+ * indicates a list of tablespaceinfo structs describing the cluster's
+ * tablespaces.).
  *
  * The tablespace map contents are appended to passed-in parameter
  * tablespace_map and the caller is responsible for including it in the backup
@@ -8269,11 +8269,11 @@ issue_xlog_fsync(int fd, XLogSegNo segno, TimeLineID 
tli)
  */
 void
 do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces,
-                                  BackupState *state, StringInfo tblspcmapfile)
+                                  BackupState *backup_state, StringInfo 
tblspcmapfile)
 {
        bool            backup_started_in_recovery = false;
 
-       Assert(state != NULL);
+       Assert(backup_state != NULL);
        backup_started_in_recovery = RecoveryInProgress();
 
        /*
@@ -8292,7 +8292,7 @@ do_pg_backup_start(const char *backupidstr, bool fast, 
List **tablespaces,
                                 errmsg("backup label too long (max %d bytes)",
                                                MAXPGPATH)));
 
-       memcpy(state->name, backupidstr, strlen(backupidstr));
+       memcpy(backup_state->name, backupidstr, strlen(backupidstr));
 
        /*
         * Mark backup active in shared memory.  We must do full-page WAL writes
@@ -8385,9 +8385,9 @@ do_pg_backup_start(const char *backupidstr, bool fast, 
List **tablespaces,
                         * pointer.
                         */
                        LWLockAcquire(ControlFileLock, LW_SHARED);
-                       state->checkpointloc = ControlFile->checkPoint;
-                       state->startpoint = ControlFile->checkPointCopy.redo;
-                       state->starttli = 
ControlFile->checkPointCopy.ThisTimeLineID;
+                       backup_state->checkpointloc = ControlFile->checkPoint;
+                       backup_state->startpoint = 
ControlFile->checkPointCopy.redo;
+                       backup_state->starttli = 
ControlFile->checkPointCopy.ThisTimeLineID;
                        checkpointfpw = 
ControlFile->checkPointCopy.fullPageWrites;
                        LWLockRelease(ControlFileLock);
 
@@ -8404,7 +8404,7 @@ do_pg_backup_start(const char *backupidstr, bool fast, 
List **tablespaces,
                                recptr = XLogCtl->lastFpwDisableRecPtr;
                                SpinLockRelease(&XLogCtl->info_lck);
 
-                               if (!checkpointfpw || state->startpoint <= 
recptr)
+                               if (!checkpointfpw || backup_state->startpoint 
<= recptr)
                                        ereport(ERROR,
                                                        
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
                                                         errmsg("WAL generated 
with full_page_writes=off was replayed "
@@ -8436,9 +8436,9 @@ do_pg_backup_start(const char *backupidstr, bool fast, 
List **tablespaces,
                         * either because only few buffers have been dirtied 
yet.
                         */
                        WALInsertLockAcquireExclusive();
-                       if (XLogCtl->Insert.lastBackupStart < state->startpoint)
+                       if (XLogCtl->Insert.lastBackupStart < 
backup_state->startpoint)
                        {
-                               XLogCtl->Insert.lastBackupStart = 
state->startpoint;
+                               XLogCtl->Insert.lastBackupStart = 
backup_state->startpoint;
                                gotUniqueStartpoint = true;
                        }
                        WALInsertLockRelease();
@@ -8529,11 +8529,11 @@ do_pg_backup_start(const char *backupidstr, bool fast, 
List **tablespaces,
                }
                FreeDir(tblspcdir);
 
-               state->starttime = (pg_time_t) time(NULL);
+               backup_state->starttime = (pg_time_t) time(NULL);
        }
        PG_END_ENSURE_ERROR_CLEANUP(pg_backup_start_callback, (Datum) 0);
 
-       state->started_in_recovery = backup_started_in_recovery;
+       backup_state->started_in_recovery = backup_started_in_recovery;
 
        /*
         * Mark that the start phase has correctly finished for the backup.
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index dce265098e..9e01146c6c 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -279,7 +279,7 @@ typedef enum SessionBackupState
 } SessionBackupState;
 
 extern void do_pg_backup_start(const char *backupidstr, bool fast,
-                                                          List **tablespaces, 
BackupState *state,
+                                                          List **tablespaces, 
BackupState *backup_state,
                                                           StringInfo 
tblspcmapfile);
 extern void do_pg_backup_stop(BackupState *state, bool waitforarchive);
 extern void do_pg_abort_backup(int code, Datum arg);

Reply via email to