From 4170298b6ecff7ce697b81e13d9a81e3b825798c Mon Sep 17 00:00:00 2001 From: Andrew Dunstan Date: Thu, 10 Apr 2025 12:11:36 -0400 Subject: [PATCH] Further cleanup for directory creation on pg_dump/pg_dumpall Instead of two separate (and different) implementations, refactor to use a single common routine. Along the way, remove use of a hardcoded file permissions constant in favor of the common project setting for directory creation. Author: Mahendra Singh Thalor Discussion: https://postgr.es/m/CAKYtNApihL8X1h7XO-zOjznc8Ca66Aevgvhc9zOTh6DBh2iaeA@mail.gmail.com --- src/bin/pg_dump/dumputils.c | 36 +++++++++++++++++++++++++ src/bin/pg_dump/dumputils.h | 1 + src/bin/pg_dump/pg_backup_directory.c | 36 ++----------------------- src/bin/pg_dump/pg_dumpall.c | 39 +-------------------------- 4 files changed, 40 insertions(+), 72 deletions(-) diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c index ab0e9e6da3c..73ce34346b2 100644 --- a/src/bin/pg_dump/dumputils.c +++ b/src/bin/pg_dump/dumputils.c @@ -16,6 +16,8 @@ #include +#include "common/file_perm.h" +#include "common/logging.h" #include "dumputils.h" #include "fe_utils/string_utils.h" @@ -884,3 +886,37 @@ makeAlterConfigCommand(PGconn *conn, const char *configitem, pg_free(mine); } + +/* + * create_or_open_dir + * + * This will create a new directory with the given dirname. If there is + * already an empty directory with that name, then use it. + */ +void +create_or_open_dir(const char *dirname) +{ + int ret; + + switch ((ret = pg_check_dir(dirname))) + { + case -1: + /* opendir failed but not with ENOENT */ + pg_fatal("could not open directory \"%s\": %m", dirname); + break; + case 0: + /* directory does not exist */ + if (mkdir(dirname, pg_dir_create_mode) < 0) + pg_fatal("could not create directory \"%s\": %m", dirname); + break; + case 1: + /* exists and is empty, fix perms */ + if (chmod(dirname, pg_dir_create_mode) != 0) + pg_fatal("could not change permissions of directory \"%s\": %m", + dirname); + break; + default: + /* exists and is not empty */ + pg_fatal("directory \"%s\" is not empty", dirname); + } +} diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h index a4bd16857ce..91c6e612e28 100644 --- a/src/bin/pg_dump/dumputils.h +++ b/src/bin/pg_dump/dumputils.h @@ -62,5 +62,6 @@ extern void makeAlterConfigCommand(PGconn *conn, const char *configitem, const char *type, const char *name, const char *type2, const char *name2, PQExpBuffer buf); +extern void create_or_open_dir(const char *dirname); #endif /* DUMPUTILS_H */ diff --git a/src/bin/pg_dump/pg_backup_directory.c b/src/bin/pg_dump/pg_backup_directory.c index b2a841bb0ff..21b00792a8a 100644 --- a/src/bin/pg_dump/pg_backup_directory.c +++ b/src/bin/pg_dump/pg_backup_directory.c @@ -41,6 +41,7 @@ #include "common/file_utils.h" #include "compress_io.h" +#include "dumputils.h" #include "parallel.h" #include "pg_backup_utils.h" @@ -156,41 +157,8 @@ InitArchiveFmt_Directory(ArchiveHandle *AH) if (AH->mode == archModeWrite) { - struct stat st; - bool is_empty = false; - /* we accept an empty existing directory */ - if (stat(ctx->directory, &st) == 0 && S_ISDIR(st.st_mode)) - { - DIR *dir = opendir(ctx->directory); - - if (dir) - { - struct dirent *d; - - is_empty = true; - while (errno = 0, (d = readdir(dir))) - { - if (strcmp(d->d_name, ".") != 0 && strcmp(d->d_name, "..") != 0) - { - is_empty = false; - break; - } - } - - if (errno) - pg_fatal("could not read directory \"%s\": %m", - ctx->directory); - - if (closedir(dir)) - pg_fatal("could not close directory \"%s\": %m", - ctx->directory); - } - } - - if (!is_empty && mkdir(ctx->directory, 0700) < 0) - pg_fatal("could not create directory \"%s\": %m", - ctx->directory); + create_or_open_dir(ctx->directory); } else { /* Read Mode */ diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c index 18b544b0214..3395d559518 100644 --- a/src/bin/pg_dump/pg_dumpall.c +++ b/src/bin/pg_dump/pg_dumpall.c @@ -15,7 +15,6 @@ #include "postgres_fe.h" -#include #include #include @@ -78,7 +77,6 @@ static void executeCommand(PGconn *conn, const char *query); static void expand_dbname_patterns(PGconn *conn, SimpleStringList *patterns, SimpleStringList *names); static void read_dumpall_filters(const char *filename, SimpleStringList *pattern); -static void create_or_open_dir(const char *dirname); static ArchiveFormat parseDumpFormat(const char *format); static char pg_dump_bin[MAXPGPATH]; @@ -1655,7 +1653,7 @@ dumpDatabases(PGconn *conn, ArchiveFormat archDumpFormat) snprintf(db_subdir, MAXPGPATH, "%s/databases", filename); /* Create a subdirectory with 'databases' name under main directory. */ - if (mkdir(db_subdir, 0755) != 0) + if (mkdir(db_subdir, pg_dir_create_mode) != 0) pg_fatal("could not create subdirectory \"%s\": %m", db_subdir); snprintf(map_file_path, MAXPGPATH, "%s/map.dat", filename); @@ -1946,41 +1944,6 @@ read_dumpall_filters(const char *filename, SimpleStringList *pattern) filter_free(&fstate); } -/* - * create_or_open_dir - * - * This will create a new directory with the given dirname. If there is - * already an empty directory with that name, then use it. - */ -static void -create_or_open_dir(const char *dirname) -{ - int ret; - - switch ((ret = pg_check_dir(dirname))) - { - case -1: - /* opendir failed but not with ENOENT */ - pg_fatal("could not open directory \"%s\": %m", dirname); - break; - case 0: - /* directory does not exist */ - if (mkdir(dirname, pg_dir_create_mode) < 0) - pg_fatal("could not create directory \"%s\": %m", dirname); - break; - case 1: - /* exists and is empty, fix perms */ - if (chmod(dirname, pg_dir_create_mode) != 0) - pg_fatal("could not change permissions of directory \"%s\": %m", - dirname); - - break; - default: - /* exists and is not empty */ - pg_fatal("directory \"%s\" is not empty", dirname); - } -} - /* * parseDumpFormat *