Switch pg_verify_checksums back to a blacklist

michaelpq · michaelpq · commit a1c91dd1108c · 2018-11-30T10:14:58.000+09:00
This basically reverts commit d55241a, 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.GA14282@paquier.xyz Backpatch-through: 11
diff --git a/src/bin/pg_verify_checksums/pg_verify_checksums.c b/src/bin/pg_verify_checksums/pg_verify_checksums.c
@@ -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 <pgsql-bugs@postgresql.org>.\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);
diff --git a/src/bin/pg_verify_checksums/t/002_actions.pl b/src/bin/pg_verify_checksums/t/002_actions.pl
@@ -17,23 +17,6 @@
 	     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", "";
