From a92ccf47c9c334eea5b8d07b8fcab7031181c37e Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Tue, 15 Feb 2022 09:40:53 -0800 Subject: [PATCH v12 1/2] make more use of get_dirent_type() --- src/backend/access/heap/rewriteheap.c | 4 +- .../replication/logical/reorderbuffer.c | 12 +++--- src/backend/replication/logical/snapbuild.c | 5 +-- src/backend/replication/slot.c | 4 +- src/backend/storage/file/copydir.c | 21 +++------- src/backend/storage/file/fd.c | 20 +++++---- src/backend/utils/misc/guc-file.l | 42 +++++++------------ src/timezone/pgtz.c | 8 +--- 8 files changed, 48 insertions(+), 68 deletions(-) diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 2a53826736..d64d7aae2e 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -113,6 +113,7 @@ #include "access/xact.h" #include "access/xloginsert.h" #include "catalog/catalog.h" +#include "common/file_utils.h" #include "lib/ilist.h" #include "miscadmin.h" #include "pgstat.h" @@ -1213,7 +1214,6 @@ CheckPointLogicalRewriteHeap(void) mappings_dir = AllocateDir("pg_logical/mappings"); while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) { - struct stat statbuf; Oid dboid; Oid relid; XLogRecPtr lsn; @@ -1227,7 +1227,7 @@ CheckPointLogicalRewriteHeap(void) continue; snprintf(path, sizeof(path), "pg_logical/mappings/%s", mapping_de->d_name); - if (lstat(path, &statbuf) == 0 && !S_ISREG(statbuf.st_mode)) + if (get_dirent_type(path, mapping_de, false, ERROR) != PGFILETYPE_REG) continue; /* Skip over files that cannot be ours. */ diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c index 5adc016d44..63ef55f3f7 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -91,6 +91,7 @@ #include "access/xact.h" #include "access/xlog_internal.h" #include "catalog/catalog.h" +#include "common/file_utils.h" #include "lib/binaryheap.h" #include "miscadmin.h" #include "pgstat.h" @@ -4408,15 +4409,10 @@ ReorderBufferCleanupSerializedTXNs(const char *slotname) { DIR *spill_dir; struct dirent *spill_de; - struct stat statbuf; char path[MAXPGPATH * 2 + 12]; sprintf(path, "pg_replslot/%s", slotname); - /* we're only handling directories here, skip if it's not ours */ - if (lstat(path, &statbuf) == 0 && !S_ISDIR(statbuf.st_mode)) - return; - spill_dir = AllocateDir(path); while ((spill_de = ReadDirExtended(spill_dir, path, INFO)) != NULL) { @@ -4464,6 +4460,7 @@ StartupReorderBuffer(void) { DIR *logical_dir; struct dirent *logical_de; + char path[MAXPGPATH * 2 + 12]; logical_dir = AllocateDir("pg_replslot"); while ((logical_de = ReadDir(logical_dir, "pg_replslot")) != NULL) @@ -4476,6 +4473,11 @@ StartupReorderBuffer(void) if (!ReplicationSlotValidateName(logical_de->d_name, DEBUG2)) continue; + /* we're only handling directories here, skip if it's not ours */ + sprintf(path, "pg_replslot/%s", logical_de->d_name); + if (get_dirent_type(path, logical_de, false, ERROR) != PGFILETYPE_DIR) + continue; + /* * ok, has to be a surviving logical slot, iterate and delete * everything starting with xid-* diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 83fca8a77d..df50abfd98 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -123,6 +123,7 @@ #include "access/heapam_xlog.h" #include "access/transam.h" #include "access/xact.h" +#include "common/file_utils.h" #include "miscadmin.h" #include "pgstat.h" #include "replication/logical.h" @@ -1947,15 +1948,13 @@ CheckPointSnapBuild(void) uint32 hi; uint32 lo; XLogRecPtr lsn; - struct stat statbuf; if (strcmp(snap_de->d_name, ".") == 0 || strcmp(snap_de->d_name, "..") == 0) continue; snprintf(path, sizeof(path), "pg_logical/snapshots/%s", snap_de->d_name); - - if (lstat(path, &statbuf) == 0 && !S_ISREG(statbuf.st_mode)) + if (get_dirent_type(path, snap_de, false, ERROR) != PGFILETYPE_REG) { elog(DEBUG1, "only regular files expected: %s", path); continue; diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c index c35ea7c35b..5ee68e71b8 100644 --- a/src/backend/replication/slot.c +++ b/src/backend/replication/slot.c @@ -41,6 +41,7 @@ #include "access/transam.h" #include "access/xlog_internal.h" +#include "common/file_utils.h" #include "common/string.h" #include "miscadmin.h" #include "pgstat.h" @@ -1485,7 +1486,6 @@ StartupReplicationSlots(void) replication_dir = AllocateDir("pg_replslot"); while ((replication_de = ReadDir(replication_dir, "pg_replslot")) != NULL) { - struct stat statbuf; char path[MAXPGPATH + 12]; if (strcmp(replication_de->d_name, ".") == 0 || @@ -1495,7 +1495,7 @@ StartupReplicationSlots(void) snprintf(path, sizeof(path), "pg_replslot/%s", replication_de->d_name); /* we're only creating directories here, skip if it's not our's */ - if (lstat(path, &statbuf) == 0 && !S_ISDIR(statbuf.st_mode)) + if (get_dirent_type(path, replication_de, false, ERROR) != PGFILETYPE_DIR) continue; /* we crashed while a slot was being setup or deleted, clean up */ diff --git a/src/backend/storage/file/copydir.c b/src/backend/storage/file/copydir.c index 658fd95ba9..8a866191e1 100644 --- a/src/backend/storage/file/copydir.c +++ b/src/backend/storage/file/copydir.c @@ -22,6 +22,7 @@ #include #include +#include "common/file_utils.h" #include "miscadmin.h" #include "pgstat.h" #include "storage/copydir.h" @@ -50,7 +51,7 @@ copydir(char *fromdir, char *todir, bool recurse) while ((xlde = ReadDir(xldir, fromdir)) != NULL) { - struct stat fst; + PGFileType xlde_type; /* If we got a cancel signal during the copy of the directory, quit */ CHECK_FOR_INTERRUPTS(); @@ -62,18 +63,15 @@ copydir(char *fromdir, char *todir, bool recurse) snprintf(fromfile, sizeof(fromfile), "%s/%s", fromdir, xlde->d_name); snprintf(tofile, sizeof(tofile), "%s/%s", todir, xlde->d_name); - if (lstat(fromfile, &fst) < 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not stat file \"%s\": %m", fromfile))); + xlde_type = get_dirent_type(fromfile, xlde, false, ERROR); - if (S_ISDIR(fst.st_mode)) + if (xlde_type == PGFILETYPE_DIR) { /* recurse to handle subdirectories */ if (recurse) copydir(fromfile, tofile, true); } - else if (S_ISREG(fst.st_mode)) + else if (xlde_type == PGFILETYPE_REG) copy_file(fromfile, tofile); } FreeDir(xldir); @@ -89,8 +87,6 @@ copydir(char *fromdir, char *todir, bool recurse) while ((xlde = ReadDir(xldir, todir)) != NULL) { - struct stat fst; - if (strcmp(xlde->d_name, ".") == 0 || strcmp(xlde->d_name, "..") == 0) continue; @@ -101,12 +97,7 @@ copydir(char *fromdir, char *todir, bool recurse) * We don't need to sync subdirectories here since the recursive * copydir will do it before it returns */ - if (lstat(tofile, &fst) < 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not stat file \"%s\": %m", tofile))); - - if (S_ISREG(fst.st_mode)) + if (get_dirent_type(tofile, xlde, false, ERROR) == PGFILETYPE_REG) fsync_fname(tofile, false); } FreeDir(xldir); diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c index 14b77f2861..4a4a79b40a 100644 --- a/src/backend/storage/file/fd.c +++ b/src/backend/storage/file/fd.c @@ -2783,6 +2783,10 @@ TryAgain: * * The pathname passed to AllocateDir must be passed to this routine too, * but it is only used for error reporting. + * + * If you need to discover the file type of an entry, consider using + * get_dirent_type() (instead of stat()/lstat()) to avoid extra system calls on + * many platforms. */ struct dirent * ReadDir(DIR *dir, const char *dirname) @@ -2798,6 +2802,10 @@ ReadDir(DIR *dir, const char *dirname) * If elevel < ERROR, returns NULL after any error. With the normal coding * pattern, this will result in falling out of the loop immediately as * though the directory contained no (more) entries. + * + * If you need to discover the file type of an entry, consider using + * get_dirent_type() (instead of stat()/lstat()) to avoid extra system calls on + * many platforms. */ struct dirent * ReadDirExtended(DIR *dir, const char *dirname, int elevel) @@ -3234,17 +3242,11 @@ RemovePgTempFilesInDir(const char *tmpdirname, bool missing_ok, bool unlink_all) PG_TEMP_FILE_PREFIX, strlen(PG_TEMP_FILE_PREFIX)) == 0) { - struct stat statbuf; + PGFileType type = get_dirent_type(rm_path, temp_de, false, LOG); - if (lstat(rm_path, &statbuf) < 0) - { - ereport(LOG, - (errcode_for_file_access(), - errmsg("could not stat file \"%s\": %m", rm_path))); + if (type == PGFILETYPE_ERROR) continue; - } - - if (S_ISDIR(statbuf.st_mode)) + else if (type == PGFILETYPE_DIR) { /* recursively remove contents, then directory itself */ RemovePgTempFilesInDir(rm_path, false, true); diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l index c70543fa74..8f10252fa4 100644 --- a/src/backend/utils/misc/guc-file.l +++ b/src/backend/utils/misc/guc-file.l @@ -14,6 +14,7 @@ #include #include +#include "common/file_utils.h" #include "mb/pg_wchar.h" #include "miscadmin.h" #include "storage/fd.h" @@ -1018,7 +1019,7 @@ ParseConfigDirectory(const char *includedir, while ((de = ReadDir(d, directory)) != NULL) { - struct stat st; + PGFileType de_type; char filename[MAXPGPATH]; /* @@ -1035,32 +1036,9 @@ ParseConfigDirectory(const char *includedir, join_path_components(filename, directory, de->d_name); canonicalize_path(filename); - if (stat(filename, &st) == 0) + de_type = get_dirent_type(filename, de, true, elevel); + if (de_type == PGFILETYPE_ERROR) { - if (!S_ISDIR(st.st_mode)) - { - /* Add file to array, increasing its size in blocks of 32 */ - if (num_filenames >= size_filenames) - { - size_filenames += 32; - filenames = (char **) repalloc(filenames, - size_filenames * sizeof(char *)); - } - filenames[num_filenames] = pstrdup(filename); - num_filenames++; - } - } - else - { - /* - * stat does not care about permissions, so the most likely reason - * a file can't be accessed now is if it was removed between the - * directory listing and now. - */ - ereport(elevel, - (errcode_for_file_access(), - errmsg("could not stat file \"%s\": %m", - filename))); record_config_file_error(psprintf("could not stat file \"%s\"", filename), calling_file, calling_lineno, @@ -1068,6 +1046,18 @@ ParseConfigDirectory(const char *includedir, status = false; goto cleanup; } + else if (de_type != PGFILETYPE_DIR) + { + /* Add file to array, increasing its size in blocks of 32 */ + if (num_filenames >= size_filenames) + { + size_filenames += 32; + filenames = (char **) repalloc(filenames, + size_filenames * sizeof(char *)); + } + filenames[num_filenames] = pstrdup(filename); + num_filenames++; + } } if (num_filenames > 0) diff --git a/src/timezone/pgtz.c b/src/timezone/pgtz.c index 3c278dd0f3..72f9e3d5e6 100644 --- a/src/timezone/pgtz.c +++ b/src/timezone/pgtz.c @@ -17,6 +17,7 @@ #include #include +#include "common/file_utils.h" #include "datatype/timestamp.h" #include "miscadmin.h" #include "pgtz.h" @@ -429,7 +430,6 @@ pg_tzenumerate_next(pg_tzenum *dir) { struct dirent *direntry; char fullname[MAXPGPATH * 2]; - struct stat statbuf; direntry = ReadDir(dir->dirdesc[dir->depth], dir->dirname[dir->depth]); @@ -447,12 +447,8 @@ pg_tzenumerate_next(pg_tzenum *dir) snprintf(fullname, sizeof(fullname), "%s/%s", dir->dirname[dir->depth], direntry->d_name); - if (stat(fullname, &statbuf) != 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not stat \"%s\": %m", fullname))); - if (S_ISDIR(statbuf.st_mode)) + if (get_dirent_type(fullname, direntry, true, ERROR) == PGFILETYPE_DIR) { /* Step into the subdirectory */ if (dir->depth >= MAX_TZDIR_DEPTH - 1) -- 2.25.1