Hi all,

While looking at the code of pg_archivecleanup.c, I noticed that there
is some code present to detect if a given string has the format of a
WAL segment file name or of a backup file.
The recent commit 179cdd09 has introduced in xlog_internal.h a set of
macros to facilitate checks of pg_xlog's name format:
IsPartialXLogFileName(), IsXLogFileName() and IsTLHistoryFileName().

And by looking at the code, there are some utilities where we could
make use of that, like pg_resetxlog, pg_archivecleanup and pg_standby.

Attached is a small refactoring patch doing so for HEAD.
Regards,
-- 
Michael
diff --git a/contrib/pg_standby/pg_standby.c b/contrib/pg_standby/pg_standby.c
index 2f9f2b4..4b78209 100644
--- a/contrib/pg_standby/pg_standby.c
+++ b/contrib/pg_standby/pg_standby.c
@@ -32,6 +32,8 @@
 
 #include "pg_getopt.h"
 
+#include "access/xlog_internal.h"
+
 const char *progname;
 
 /* Options and defaults */
@@ -113,9 +115,8 @@ struct stat stat_buf;
  *	folded in to later versions of this program.
  */
 
-#define XLOG_DATA_FNAME_LEN		24
 /* Reworked from access/xlog_internal.h */
-#define XLogFileName(fname, tli, log, seg)	\
+#define XLogFileNameExtended(fname, tli, log, seg)	\
 	snprintf(fname, XLOG_DATA_FNAME_LEN + 1, "%08X%08X%08X", tli, log, seg)
 
 /*
@@ -182,10 +183,7 @@ CustomizableNextWALFileReady()
 		 * If it's a backup file, return immediately. If it's a regular file
 		 * return only if it's the right size already.
 		 */
-		if (strlen(nextWALFileName) > 24 &&
-			strspn(nextWALFileName, "0123456789ABCDEF") == 24 &&
-		strcmp(nextWALFileName + strlen(nextWALFileName) - strlen(".backup"),
-			   ".backup") == 0)
+		if (IsBackupHistoryFileName(nextWALFileName))
 		{
 			nextWALFileType = XLOG_BACKUP_LABEL;
 			return true;
@@ -261,8 +259,7 @@ CustomizableCleanupPriorWALFiles(void)
 				 * are not removed in the order they were originally written,
 				 * in case this worries you.
 				 */
-				if (strlen(xlde->d_name) == XLOG_DATA_FNAME_LEN &&
-					strspn(xlde->d_name, "0123456789ABCDEF") == XLOG_DATA_FNAME_LEN &&
+				if (IsXLogFileName(xlde->d_name) &&
 				  strcmp(xlde->d_name + 8, exclusiveCleanupFileName + 8) < 0)
 				{
 #ifdef WIN32
@@ -366,7 +363,7 @@ SetWALFileNameForCleanup(void)
 		}
 	}
 
-	XLogFileName(exclusiveCleanupFileName, tli, log, seg);
+	XLogFileNameExtended(exclusiveCleanupFileName, tli, log, seg);
 
 	return cleanup;
 }
@@ -740,10 +737,7 @@ main(int argc, char **argv)
 	 * Check for initial history file: always the first file to be requested
 	 * It's OK if the file isn't there - all other files need to wait
 	 */
-	if (strlen(nextWALFileName) > 8 &&
-		strspn(nextWALFileName, "0123456789ABCDEF") == 8 &&
-		strcmp(nextWALFileName + strlen(nextWALFileName) - strlen(".history"),
-			   ".history") == 0)
+	if (IsTLHistoryFileName(nextWALFileName))
 	{
 		nextWALFileType = XLOG_HISTORY;
 		if (RestoreWALFileForRecovery())
diff --git a/src/bin/pg_archivecleanup/pg_archivecleanup.c b/src/bin/pg_archivecleanup/pg_archivecleanup.c
index ba6e242..ded36cc 100644
--- a/src/bin/pg_archivecleanup/pg_archivecleanup.c
+++ b/src/bin/pg_archivecleanup/pg_archivecleanup.c
@@ -21,6 +21,8 @@
 
 #include "pg_getopt.h"
 
+#include "access/xlog_internal.h"
+
 const char *progname;
 
 /* Options and defaults */
@@ -51,11 +53,9 @@ char		exclusiveCleanupFileName[MAXPGPATH];		/* the oldest file we
  *	folded in to later versions of this program.
  */
 
-#define XLOG_DATA_FNAME_LEN		24
 /* Reworked from access/xlog_internal.h */
-#define XLogFileName(fname, tli, log, seg)	\
+#define XLogFileNameExtended(fname, tli, log, seg)	\
 	snprintf(fname, XLOG_DATA_FNAME_LEN + 1, "%08X%08X%08X", tli, log, seg)
-#define XLOG_BACKUP_FNAME_LEN	40
 
 /*
  *	Initialize allows customized commands into the archive cleanup program.
@@ -129,8 +129,7 @@ CleanupPriorWALFiles(void)
 			 * file. Note that this means files are not removed in the order
 			 * they were originally written, in case this worries you.
 			 */
-			if (strlen(walfile) == XLOG_DATA_FNAME_LEN &&
-				strspn(walfile, "0123456789ABCDEF") == XLOG_DATA_FNAME_LEN &&
+			if (IsXLogFileName(walfile) &&
 				strcmp(walfile + 8, exclusiveCleanupFileName + 8) < 0)
 			{
 				/*
@@ -202,13 +201,12 @@ SetWALFileNameForCleanup(void)
 	 * 000000010000000000000010.00000020.backup is after
 	 * 000000010000000000000010.
 	 */
-	if (strlen(restartWALFileName) == XLOG_DATA_FNAME_LEN &&
-		strspn(restartWALFileName, "0123456789ABCDEF") == XLOG_DATA_FNAME_LEN)
+	if (IsXLogFileName(restartWALFileName))
 	{
 		strcpy(exclusiveCleanupFileName, restartWALFileName);
 		fnameOK = true;
 	}
-	else if (strlen(restartWALFileName) == XLOG_BACKUP_FNAME_LEN)
+	else if (IsBackupHistoryFileName(restartWALFileName))
 	{
 		int			args;
 		uint32		tli = 1,
@@ -225,7 +223,7 @@ SetWALFileNameForCleanup(void)
 			 * Use just the prefix of the filename, ignore everything after
 			 * first period
 			 */
-			XLogFileName(exclusiveCleanupFileName, tli, log, seg);
+			XLogFileNameExtended(exclusiveCleanupFileName, tli, log, seg);
 		}
 	}
 
diff --git a/src/bin/pg_resetxlog/pg_resetxlog.c b/src/bin/pg_resetxlog/pg_resetxlog.c
index 6ffe795..81fcaac 100644
--- a/src/bin/pg_resetxlog/pg_resetxlog.c
+++ b/src/bin/pg_resetxlog/pg_resetxlog.c
@@ -261,7 +261,7 @@ main(int argc, char *argv[])
 				break;
 
 			case 'l':
-				if (strspn(optarg, "01234567890ABCDEFabcdef") != 24)
+				if (strspn(optarg, "01234567890ABCDEFabcdef") != XLOG_DATA_FNAME_LEN)
 				{
 					fprintf(stderr, _("%s: invalid argument for option %s\n"), progname, "-l");
 					fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
@@ -976,8 +976,7 @@ KillExistingXLOG(void)
 
 	while (errno = 0, (xlde = readdir(xldir)) != NULL)
 	{
-		if (strlen(xlde->d_name) == 24 &&
-			strspn(xlde->d_name, "0123456789ABCDEF") == 24)
+		if (IsXLogFileName(xlde->d_name))
 		{
 			snprintf(path, MAXPGPATH, "%s/%s", XLOGDIR, xlde->d_name);
 			if (unlink(path) < 0)
@@ -1027,9 +1026,9 @@ KillExistingArchiveStatus(void)
 
 	while (errno = 0, (xlde = readdir(xldir)) != NULL)
 	{
-		if (strspn(xlde->d_name, "0123456789ABCDEF") == 24 &&
-			(strcmp(xlde->d_name + 24, ".ready") == 0 ||
-			 strcmp(xlde->d_name + 24, ".done") == 0))
+		if (strspn(xlde->d_name, "0123456789ABCDEF") == XLOG_DATA_FNAME_LEN &&
+			(strcmp(xlde->d_name + XLOG_DATA_FNAME_LEN, ".ready") == 0 ||
+			 strcmp(xlde->d_name + XLOG_DATA_FNAME_LEN, ".done") == 0))
 		{
 			snprintf(path, MAXPGPATH, "%s/%s", ARCHSTATDIR, xlde->d_name);
 			if (unlink(path) < 0)
diff --git a/src/include/access/xlog_internal.h b/src/include/access/xlog_internal.h
index fbf9324..a72a80d 100644
--- a/src/include/access/xlog_internal.h
+++ b/src/include/access/xlog_internal.h
@@ -135,7 +135,10 @@ typedef XLogLongPageHeaderData *XLogLongPageHeader;
  * These macros encapsulate knowledge about the exact layout of XLog file
  * names, timeline history file names, and archive-status file names.
  */
-#define MAXFNAMELEN		64
+#define MAXFNAMELEN				64
+
+/* Length of XLog file name */
+#define XLOG_DATA_FNAME_LEN     24
 
 #define XLogFileName(fname, tli, logSegNo)	\
 	snprintf(fname, MAXFNAMELEN, "%08X%08X%08X", tli,		\
@@ -143,7 +146,8 @@ typedef XLogLongPageHeaderData *XLogLongPageHeader;
 			 (uint32) ((logSegNo) % XLogSegmentsPerXLogId))
 
 #define IsXLogFileName(fname) \
-	(strlen(fname) == 24 && strspn(fname, "0123456789ABCDEF") == 24)
+	(strlen(fname) == XLOG_DATA_FNAME_LEN && \
+	 strspn(fname, "0123456789ABCDEF") == XLOG_DATA_FNAME_LEN)
 
 /*
  * XLOG segment with .partial suffix.  Used by pg_receivexlog and at end of
@@ -151,9 +155,9 @@ typedef XLogLongPageHeaderData *XLogLongPageHeader;
  * be complete yet.
  */
 #define IsPartialXLogFileName(fname)	\
-	(strlen(fname) == 24 + strlen(".partial") &&	\
-	 strspn(fname, "0123456789ABCDEF") == 24 &&		\
-	 strcmp((fname) + 24, ".partial") == 0)
+	(strlen(fname) == XLOG_DATA_FNAME_LEN + strlen(".partial") &&	\
+	 strspn(fname, "0123456789ABCDEF") == XLOG_DATA_FNAME_LEN &&		\
+	 strcmp((fname) + XLOG_DATA_FNAME_LEN, ".partial") == 0)
 
 #define XLogFromFileName(fname, tli, logSegNo)	\
 	do {												\
@@ -188,8 +192,8 @@ typedef XLogLongPageHeaderData *XLogLongPageHeader;
 			 (uint32) ((logSegNo) % XLogSegmentsPerXLogId), offset)
 
 #define IsBackupHistoryFileName(fname) \
-	(strlen(fname) > 24 && \
-	 strspn(fname, "0123456789ABCDEF") == 24 && \
+	(strlen(fname) > XLOG_DATA_FNAME_LEN && \
+	 strspn(fname, "0123456789ABCDEF") == XLOG_DATA_FNAME_LEN && \
 	 strcmp((fname) + strlen(fname) - strlen(".backup"), ".backup") == 0)
 
 #define BackupHistoryFilePath(path, tli, logSegNo, offset)	\
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to