Fix CRC check handling in get_controlfile

The previous patch broke this by returning NULL for a failed CRC check,
which pg_controldata would then try to read.  Fix by returning the
result of the CRC check in a separate argument.

Michael Paquier and myself

diff --git a/src/backend/utils/misc/pg_controldata.c b/src/backend/utils/misc/pg_controldata.c
index 4f9de830974456adaafc7b5cbe55455660c7c46f..0c67833f60b0fada001d4ce4d337c66d71404675 100644 (file)
--- a/src/backend/utils/misc/pg_controldata.c
+++ b/src/backend/utils/misc/pg_controldata.c
@@ -34,6 +34,7 @@ pg_control_system(PG_FUNCTION_ARGS)
        TupleDesc       tupdesc;
        HeapTuple       htup;
        ControlFileData *ControlFile;
+       bool            crc_ok;
 
        /*
         * Construct a tuple descriptor for the result row.  This must match this
@@ -51,8 +52,8 @@ pg_control_system(PG_FUNCTION_ARGS)
        tupdesc = BlessTupleDesc(tupdesc);
 
        /* read the control file */
-       ControlFile = get_controlfile(DataDir, NULL);
-       if (!ControlFile)
+       ControlFile = get_controlfile(DataDir, NULL, &crc_ok);
+       if (!crc_ok)
                ereport(ERROR,
                                (errmsg("calculated CRC checksum does not match value stored in file")));
 
@@ -83,6 +84,7 @@ pg_control_checkpoint(PG_FUNCTION_ARGS)
        ControlFileData *ControlFile;
        XLogSegNo       segno;
        char            xlogfilename[MAXFNAMELEN];
+       bool            crc_ok;
 
        /*
         * Construct a tuple descriptor for the result row.  This must match this
@@ -130,8 +132,8 @@ pg_control_checkpoint(PG_FUNCTION_ARGS)
        tupdesc = BlessTupleDesc(tupdesc);
 
        /* Read the control file. */
-       ControlFile = get_controlfile(DataDir, NULL);
-       if (!ControlFile)
+       ControlFile = get_controlfile(DataDir, NULL, &crc_ok);
+       if (!crc_ok)
                ereport(ERROR,
                                (errmsg("calculated CRC checksum does not match value stored in file")));
 
@@ -216,6 +218,7 @@ pg_control_recovery(PG_FUNCTION_ARGS)
        TupleDesc       tupdesc;
        HeapTuple       htup;
        ControlFileData *ControlFile;
+       bool            crc_ok;
 
        /*
         * Construct a tuple descriptor for the result row.  This must match this
@@ -235,8 +238,8 @@ pg_control_recovery(PG_FUNCTION_ARGS)
        tupdesc = BlessTupleDesc(tupdesc);
 
        /* read the control file */
-       ControlFile = get_controlfile(DataDir, NULL);
-       if (!ControlFile)
+       ControlFile = get_controlfile(DataDir, NULL, &crc_ok);
+       if (!crc_ok)
                ereport(ERROR,
                                (errmsg("calculated CRC checksum does not match value stored in file")));
 
@@ -268,6 +271,7 @@ pg_control_init(PG_FUNCTION_ARGS)
        TupleDesc       tupdesc;
        HeapTuple       htup;
        ControlFileData *ControlFile;
+       bool            crc_ok;
 
        /*
         * Construct a tuple descriptor for the result row.  This must match this
@@ -303,8 +307,8 @@ pg_control_init(PG_FUNCTION_ARGS)
        tupdesc = BlessTupleDesc(tupdesc);
 
        /* read the control file */
-       ControlFile = get_controlfile(DataDir, NULL);
-       if (!ControlFile)
+       ControlFile = get_controlfile(DataDir, NULL, &crc_ok);
+       if (!crc_ok)
                ereport(ERROR,
                                (errmsg("calculated CRC checksum does not match value stored in file")));
 

diff --git a/src/bin/pg_controldata/pg_controldata.c b/src/bin/pg_controldata/pg_controldata.c
index e92feabadef4a005701467eec4e860e913f69d11..20077a663974544c0d385b32a244ea87be406312 100644 (file)
--- a/src/bin/pg_controldata/pg_controldata.c
+++ b/src/bin/pg_controldata/pg_controldata.c
@@ -86,6 +86,7 @@ int
 main(int argc, char *argv[])
 {
        ControlFileData *ControlFile;
+       bool            crc_ok;
        char       *DataDir = NULL;
        time_t          time_tmp;
        char            pgctime_str[128];
@@ -155,8 +156,8 @@ main(int argc, char *argv[])
        }
 
        /* get a copy of the control file */
-       ControlFile = get_controlfile(DataDir, progname);
-       if (!ControlFile)
+       ControlFile = get_controlfile(DataDir, progname, &crc_ok);
+       if (!crc_ok)
                printf(_("WARNING: Calculated CRC checksum does not match value stored in file.\n"
                                 "Either the file is corrupt, or it has a different layout than this program\n"
                                 "is expecting.  The results below are untrustworthy.\n\n"));

diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 052b02e0ff91a4ed8172a6b99eca4da0a8c079f0..ab10d2f25c059ffa7943fafa596213028ee9dc61 100644 (file)
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -2147,28 +2147,18 @@ static DBState
 get_control_dbstate(void)
 {
        DBState ret;
+       bool    crc_ok;
+       ControlFileData *control_file_data = get_controlfile(pg_data, progname, &crc_ok);
 
-       for (;;)
+       if (!crc_ok)
        {
-               ControlFileData *control_file_data = get_controlfile(pg_data, progname);
-
-               if (control_file_data)
-               {
-                       ret = control_file_data->state;
-                       pfree(control_file_data);
-                       return ret;
-               }
-
-               if (wait_seconds > 0)
-               {
-                       pg_usleep(1000000);             /* 1 sec */
-                       wait_seconds--;
-                       continue;
-               }
-
                write_stderr(_("%s: control file appears to be corrupt\n"), progname);
                exit(1);
        }
+
+       ret = control_file_data->state;
+       pfree(control_file_data);
+       return ret;
 }
 
 

diff --git a/src/common/controldata_utils.c b/src/common/controldata_utils.c
index f218d2558c196ca79ee7854c96975a554b650fd6..cbdae052a7514f77c6d20dd98ac7553c5586e729 100644 (file)
--- a/src/common/controldata_utils.c
+++ b/src/common/controldata_utils.c
@@ -29,21 +29,24 @@
 #include "port/pg_crc32c.h"
 
 /*
- * get_controlfile(char *DataDir, const char *progname)
+ * get_controlfile(char *DataDir, const char *progname, bool *crc_ok_p)
  *
- * Get controlfile values. The caller is responsible
- * for pfreeing the result.
+ * Get controlfile values.  The result is returned as a palloc'd copy of the
+ * control file data.
  *
- * Returns NULL if the CRC did not match.
+ * crc_ok_p can be used by the caller to see whether the CRC of the control
+ * file data is correct.
  */
 ControlFileData *
-get_controlfile(const char *DataDir, const char *progname)
+get_controlfile(const char *DataDir, const char *progname, bool *crc_ok_p)
 {
        ControlFileData *ControlFile;
        int                     fd;
        char            ControlFilePath[MAXPGPATH];
        pg_crc32c       crc;
 
+       AssertArg(crc_ok_p);
+
        ControlFile = palloc(sizeof(ControlFileData));
        snprintf(ControlFilePath, MAXPGPATH, "%s/global/pg_control", DataDir);
 
@@ -83,11 +86,7 @@ get_controlfile(const char *DataDir, const char *progname)
                                offsetof(ControlFileData, crc));
        FIN_CRC32C(crc);
 
-       if (!EQ_CRC32C(crc, ControlFile->crc))
-       {
-               pfree(ControlFile);
-               return NULL;
-       }
+       *crc_ok_p = EQ_CRC32C(crc, ControlFile->crc);
 
        /* Make sure the control file is valid byte order. */
        if (ControlFile->pg_control_version % 65536 == 0 &&

diff --git a/src/include/common/controldata_utils.h b/src/include/common/controldata_utils.h
index f834624e4e70b952740936858e01395f8029c4dc..a822cba1927f508195cd9ef9344c42420aef79b5 100644 (file)
--- a/src/include/common/controldata_utils.h
+++ b/src/include/common/controldata_utils.h
@@ -12,6 +12,6 @@
 
 #include "catalog/pg_control.h"
 
-extern ControlFileData *get_controlfile(const char *DataDir, const char *progname);
+extern ControlFileData *get_controlfile(const char *DataDir, const char *progname, bool *crc_ok_p);
 
 #endif   /* COMMON_CONTROLDATA_UTILS_H */