Switch pg_verify_checksums back to a blacklist
authorMichael Paquier <[email protected]>
Fri, 30 Nov 2018 01:14:58 +0000 (10:14 +0900)
committerMichael Paquier <[email protected]>
Fri, 30 Nov 2018 01:14:58 +0000 (10:14 +0900)
This basically reverts commit d55241af705667d4503638e3f77d3689fd6be31,
leaving around a portion of the regression tests still adapted with
empty relation files, and corrupted cases.  This is also proving to be
failing to check properly relation files located in a non-default
tablespace path.

Per discussion with various folks, including Stephen Frost, David
Steele, Andres Freund, Michael Banck and myself.

Reported-by: Michael Banck
Discussion: https://postgr.es/m/20181021134206[email protected]
Backpatch-through: 11

src/bin/pg_verify_checksums/pg_verify_checksums.c
src/bin/pg_verify_checksums/t/002_actions.pl

index f0e09bea20212fb389fa32e92e591e7b6fff6600..1bc020ab6cab1359e78c4f734bc76c240ecadd32 100644 (file)
@@ -15,7 +15,6 @@
 
 #include "catalog/pg_control.h"
 #include "common/controldata_utils.h"
-#include "common/relpath.h"
 #include "getopt_long.h"
 #include "pg_getopt.h"
 #include "storage/bufpage.h"
@@ -50,69 +49,27 @@ usage(void)
        printf(_("Report bugs to <[email protected]>.\n"));
 }
 
-/*
- * isRelFileName
- *
- * Check if the given file name is authorized for checksum verification.
- */
+static const char *const skip[] = {
+       "pg_control",
+       "pg_filenode.map",
+       "pg_internal.init",
+       "PG_VERSION",
+       NULL,
+};
+
 static bool
-isRelFileName(const char *fn)
+skipfile(const char *fn)
 {
-       int                     pos;
-
-       /*----------
-        * Only files including data checksums are authorized for verification.
-        * This is guessed based on the file name by reverse-engineering
-        * GetRelationPath() so make sure to update both code paths if any
-        * updates are done.  The following file name formats are allowed:
-        * <digits>
-        * <digits>.<segment>
-        * <digits>_<forkname>
-        * <digits>_<forkname>.<segment>
-        *
-        * Note that temporary files, beginning with 't', are also skipped.
-        *
-        *----------
-        */
-
-       /* A non-empty string of digits should follow */
-       for (pos = 0; isdigit((unsigned char) fn[pos]); ++pos)
-               ;
-       /* leave if no digits */
-       if (pos == 0)
-               return false;
-       /* good to go if only digits */
-       if (fn[pos] == '\0')
-               return true;
-
-       /* Authorized fork files can be scanned */
-       if (fn[pos] == '_')
-       {
-               int                     forkchar = forkname_chars(&fn[pos + 1], NULL);
-
-               if (forkchar <= 0)
-                       return false;
+       const char *const *f;
 
-               pos += forkchar + 1;
-       }
-
-       /* Check for an optional segment number */
-       if (fn[pos] == '.')
-       {
-               int                     segchar;
-
-               for (segchar = 1; isdigit((unsigned char) fn[pos + segchar]); ++segchar)
-                       ;
-
-               if (segchar <= 1)
-                       return false;
-               pos += segchar;
-       }
+       if (strcmp(fn, ".") == 0 ||
+               strcmp(fn, "..") == 0)
+               return true;
 
-       /* Now this should be the end */
-       if (fn[pos] != '\0')
-               return false;
-       return true;
+       for (f = skip; *f; f++)
+               if (strcmp(*f, fn) == 0)
+                       return true;
+       return false;
 }
 
 static void
@@ -189,7 +146,7 @@ scan_directory(const char *basedir, const char *subdir)
                char            fn[MAXPGPATH];
                struct stat st;
 
-               if (!isRelFileName(de->d_name))
+               if (skipfile(de->d_name))
                        continue;
 
                snprintf(fn, sizeof(fn), "%s/%s", path, de->d_name);
index 0e1725d9f2f70aa79ab1efe54cf2e2ff0ec8a8a8..c640cce260be9cfe1101e384addc2bb355f58c17 100644 (file)
@@ -17,23 +17,6 @@ command_like(['pg_controldata', $pgdata],
             qr/Data page checksum version:.*1/,
                 'checksums enabled in control file');
 
-# Add set of dummy files with some contents.  These should not be scanned
-# by the tool.
-
-# On Windows, file name "foo." == "foo", so skip that pattern there.
-append_to_file "$pgdata/global/123.", "foo" unless $windows_os;
-append_to_file "$pgdata/global/123_", "foo";
-append_to_file "$pgdata/global/123_.", "foo" unless $windows_os;;
-append_to_file "$pgdata/global/123.12t", "foo";
-append_to_file "$pgdata/global/foo", "foo2";
-append_to_file "$pgdata/global/t123", "bar";
-append_to_file "$pgdata/global/123a", "bar2";
-append_to_file "$pgdata/global/.123", "foobar";
-append_to_file "$pgdata/global/_fsm", "foobar2";
-append_to_file "$pgdata/global/_init", "foobar3";
-append_to_file "$pgdata/global/_vm.123", "foohoge";
-append_to_file "$pgdata/global/123_vm.123t", "foohoge2";
-
 # These are correct but empty files, so they should pass through.
 append_to_file "$pgdata/global/99999", "";
 append_to_file "$pgdata/global/99999.123", "";