From bef376b2766923339dedbf3fae9e516262b433a1 Mon Sep 17 00:00:00 2001 From: Atsushi Torikoshi Date: Tue, 14 Jun 2023 00:33:10 +0900 Subject: [PATCH v5 3/3] Refactored to reduce the nesting level. --- src/bin/pg_archivecleanup/pg_archivecleanup.c | 146 +++++++++--------- 1 file changed, 72 insertions(+), 74 deletions(-) diff --git a/src/bin/pg_archivecleanup/pg_archivecleanup.c b/src/bin/pg_archivecleanup/pg_archivecleanup.c index a7f77d9158..f475177edb 100644 --- a/src/bin/pg_archivecleanup/pg_archivecleanup.c +++ b/src/bin/pg_archivecleanup/pg_archivecleanup.c @@ -95,87 +95,85 @@ CleanupPriorWALFiles(void) struct dirent *xlde; char walfile[MAXPGPATH]; - if ((xldir = opendir(archiveLocation)) != NULL) + if ((xldir = opendir(archiveLocation)) == NULL) + pg_fatal("could not open archive location \"%s\": %m", + archiveLocation); + + while (errno = 0, (xlde = readdir(xldir)) != NULL) { - while (errno = 0, (xlde = readdir(xldir)) != NULL) + char WALFilePath[MAXPGPATH * 2]; /* the file path + * including archive */ + /* + * Truncation is essentially harmless, because we check the file + * format including the length immediately after this. + * (In principle, one could use a 1000-character additional_ext + * and get trouble.) + */ + strlcpy(walfile, xlde->d_name, MAXPGPATH); + TrimExtension(walfile, additional_ext); + + /* + * Check file name. + * + * We skip files which are not WAL file or partial WAL file. + * Also we skip backup history files when --clean-backup-history + * is not specified. + */ + if (!IsXLogFileName(walfile) && !IsPartialXLogFileName(walfile) && + !(cleanBackupHistory && IsBackupHistoryFileName(walfile))) + continue; + + /* + * Check cutoff point. + * + * We ignore the timeline part of the XLOG segment identifiers in + * deciding whether a segment is still needed. This ensures that + * we won't prematurely remove a segment from a parent timeline. + * We could probably be a little more proactive about removing + * segments of non-parent timelines, but that would be a whole lot + * more complicated. + * + * We use the alphanumeric sorting property of the filenames to + * decide which ones are earlier than the exclusiveCleanupFileName + * file. Note that this means files are not removed in the order + * they were originally written, in case this worries you. + */ + if (strcmp(walfile + 8, exclusiveCleanupFileName + 8) >= 0) + continue; + + /* + * Use the original file name again now, including any + * extension that might have been chopped off before testing + * the sequence. + */ + snprintf(WALFilePath, sizeof(WALFilePath), "%s/%s", + archiveLocation, xlde->d_name); + + if (dryrun) { - char WALFilePath[MAXPGPATH * 2]; /* the file path - * including archive */ - /* - * Truncation is essentially harmless, because we check the file - * format including the length immediately after this. - * (In principle, one could use a 1000-character additional_ext - * and get trouble.) - */ - strlcpy(walfile, xlde->d_name, MAXPGPATH); - TrimExtension(walfile, additional_ext); - /* - * Check file name. - * - * We skip files which are not WAL file or partial WAL file. - * Also we skip backup history files when --clean-backup-history - * is not specified. + * Prints the name of the file to be removed and skips the + * actual removal. The regular printout is so that the + * user can pipe the output into some other program. */ - if (!IsXLogFileName(walfile) && !IsPartialXLogFileName(walfile) && - !(cleanBackupHistory && IsBackupHistoryFileName(walfile))) - continue; - - /* - * Check cutoff point. - * - * We ignore the timeline part of the XLOG segment identifiers in - * deciding whether a segment is still needed. This ensures that - * we won't prematurely remove a segment from a parent timeline. - * We could probably be a little more proactive about removing - * segments of non-parent timelines, but that would be a whole lot - * more complicated. - * - * We use the alphanumeric sorting property of the filenames to - * decide which ones are earlier than the exclusiveCleanupFileName - * file. Note that this means files are not removed in the order - * they were originally written, in case this worries you. - */ - if (strcmp(walfile + 8, exclusiveCleanupFileName + 8) >= 0) - continue; - - /* - * Use the original file name again now, including any - * extension that might have been chopped off before testing - * the sequence. - */ - snprintf(WALFilePath, sizeof(WALFilePath), "%s/%s", - archiveLocation, xlde->d_name); - - if (dryrun) - { - /* - * Prints the name of the file to be removed and skips the - * actual removal. The regular printout is so that the - * user can pipe the output into some other program. - */ - printf("%s\n", WALFilePath); - pg_log_debug("file \"%s\" would be removed", WALFilePath); - continue; - } - - pg_log_debug("removing file \"%s\"", WALFilePath); - - rc = unlink(WALFilePath); - if (rc != 0) - pg_fatal("could not remove file \"%s\": %m", - WALFilePath); + printf("%s\n", WALFilePath); + pg_log_debug("file \"%s\" would be removed", WALFilePath); + continue; } - if (errno) - pg_fatal("could not read archive location \"%s\": %m", - archiveLocation); - if (closedir(xldir)) - pg_fatal("could not close archive location \"%s\": %m", - archiveLocation); + pg_log_debug("removing file \"%s\"", WALFilePath); + + rc = unlink(WALFilePath); + if (rc != 0) + pg_fatal("could not remove file \"%s\": %m", + WALFilePath); } - else - pg_fatal("could not open archive location \"%s\": %m", + + if (errno) + pg_fatal("could not read archive location \"%s\": %m", + archiveLocation); + if (closedir(xldir)) + pg_fatal("could not close archive location \"%s\": %m", archiveLocation); } -- 2.39.2