From 4391644be135e15ffb052594c4ae54c4d83569df Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Thu, 28 Jul 2022 15:25:10 +1200 Subject: [PATCH v3 4/4] Replace pgwin32_is_junction() with lstat(). Now that lstat() reports junction points with S_IFLNK and unlink() can unlink them, there is no need for separate code paths for Windows in a few places. The coding around pgwin32_is_junction() was a bit suspect anyway, as we never checked for errors, and we also know that errors can be spuriously reported because of transient sharing violations on this OS. The lstat()-based code tries to deal with that by retrying. Discussion: https://postgr.es/m/CA%2BhUKGLfOOeyZpm5ByVcAt7x5Pn-%3DxGRNCvgiUPVVzjFLtnY0w%40mail.gmail.com --- src/backend/commands/tablespace.c | 3 +-- src/backend/replication/basebackup.c | 12 +----------- src/backend/storage/file/fd.c | 5 ----- src/backend/utils/adt/misc.c | 7 ------- src/bin/pg_checksums/pg_checksums.c | 4 ---- src/bin/pg_rewind/file_ops.c | 4 ---- src/common/file_utils.c | 21 --------------------- src/include/port.h | 1 - src/include/port/win32_port.h | 1 - src/port/dirmod.c | 16 ---------------- 10 files changed, 2 insertions(+), 72 deletions(-) diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c index 570ce3dbd5..dc0d7befd9 100644 --- a/src/backend/commands/tablespace.c +++ b/src/backend/commands/tablespace.c @@ -805,8 +805,7 @@ destroy_tablespace_directories(Oid tablespaceoid, bool redo) /* * Try to remove the symlink. We must however deal with the possibility * that it's a directory instead of a symlink --- this could happen during - * WAL replay (see TablespaceCreateDbspace), and it is also the case on - * Windows where junction points lstat() as directories. + * WAL replay (see TablespaceCreateDbspace). * * Note: in the redo case, we'll return true if this final step fails; * there's no point in retrying it. Also, ENOENT should provoke no more diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c index 7f85071229..a048cd5f31 100644 --- a/src/backend/replication/basebackup.c +++ b/src/backend/replication/basebackup.c @@ -1322,13 +1322,7 @@ sendDir(bbsink *sink, const char *path, int basepathlen, bool sizeonly, } /* Allow symbolic links in pg_tblspc only */ - if (strcmp(path, "./pg_tblspc") == 0 && -#ifndef WIN32 - S_ISLNK(statbuf.st_mode) -#else - pgwin32_is_junction(pathbuf) -#endif - ) + if (strcmp(path, "./pg_tblspc") == 0 && S_ISLNK(statbuf.st_mode)) { #if defined(HAVE_READLINK) || defined(WIN32) char linkpath[MAXPGPATH]; @@ -1811,11 +1805,7 @@ static void convert_link_to_directory(const char *pathbuf, struct stat *statbuf) { /* If symlink, write it as a directory anyway */ -#ifndef WIN32 if (S_ISLNK(statbuf->st_mode)) -#else - if (pgwin32_is_junction(pathbuf)) -#endif statbuf->st_mode = S_IFDIR | pg_dir_create_mode; } diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c index f904f60c08..566ebe26a0 100644 --- a/src/backend/storage/file/fd.c +++ b/src/backend/storage/file/fd.c @@ -3377,7 +3377,6 @@ SyncDataDirectory(void) */ xlog_is_symlink = false; -#ifndef WIN32 { struct stat st; @@ -3389,10 +3388,6 @@ SyncDataDirectory(void) else if (S_ISLNK(st.st_mode)) xlog_is_symlink = true; } -#else - if (pgwin32_is_junction("pg_wal")) - xlog_is_symlink = true; -#endif #ifdef HAVE_SYNCFS if (recovery_init_sync_method == RECOVERY_INIT_SYNC_METHOD_SYNCFS) diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c index 89690be2ed..412039ea6b 100644 --- a/src/backend/utils/adt/misc.c +++ b/src/backend/utils/adt/misc.c @@ -283,9 +283,7 @@ pg_tablespace_location(PG_FUNCTION_ARGS) char sourcepath[MAXPGPATH]; char targetpath[MAXPGPATH]; int rllen; -#ifndef WIN32 struct stat st; -#endif /* * It's useful to apply this function to pg_class.reltablespace, wherein @@ -316,10 +314,6 @@ pg_tablespace_location(PG_FUNCTION_ARGS) * created with allow_in_place_tablespaces enabled. If a directory is * found, a relative path to the data directory is returned. */ -#ifdef WIN32 - if (!pgwin32_is_junction(sourcepath)) - PG_RETURN_TEXT_P(cstring_to_text(sourcepath)); -#else if (lstat(sourcepath, &st) < 0) { ereport(ERROR, @@ -330,7 +324,6 @@ pg_tablespace_location(PG_FUNCTION_ARGS) if (!S_ISLNK(st.st_mode)) PG_RETURN_TEXT_P(cstring_to_text(sourcepath)); -#endif /* * In presence of a link or a junction point, return the path pointing to. diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c index dc20122c89..324ccf7783 100644 --- a/src/bin/pg_checksums/pg_checksums.c +++ b/src/bin/pg_checksums/pg_checksums.c @@ -384,11 +384,7 @@ scan_directory(const char *basedir, const char *subdir, bool sizeonly) if (!sizeonly) scan_file(fn, segmentno); } -#ifndef WIN32 else if (S_ISDIR(st.st_mode) || S_ISLNK(st.st_mode)) -#else - else if (S_ISDIR(st.st_mode) || pgwin32_is_junction(fn)) -#endif { /* * If going through the entries of pg_tblspc, we assume to operate diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index 6cb288f099..7d98283c79 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -431,11 +431,7 @@ recurse_dir(const char *datadir, const char *parentpath, /* recurse to handle subdirectories */ recurse_dir(datadir, path, callback); } -#ifndef WIN32 else if (S_ISLNK(fst.st_mode)) -#else - else if (pgwin32_is_junction(fullpath)) -#endif { #if defined(HAVE_READLINK) || defined(WIN32) char link_target[MAXPGPATH]; diff --git a/src/common/file_utils.c b/src/common/file_utils.c index 966b987d64..71c232d983 100644 --- a/src/common/file_utils.c +++ b/src/common/file_utils.c @@ -79,7 +79,6 @@ fsync_pgdata(const char *pg_data, */ xlog_is_symlink = false; -#ifndef WIN32 { struct stat st; @@ -88,10 +87,6 @@ fsync_pgdata(const char *pg_data, else if (S_ISLNK(st.st_mode)) xlog_is_symlink = true; } -#else - if (pgwin32_is_junction(pg_wal)) - xlog_is_symlink = true; -#endif /* * If possible, hint to the kernel that we're soon going to fsync the data @@ -465,21 +460,5 @@ get_dirent_type(const char *path, #endif } -#if defined(WIN32) && !defined(_MSC_VER) - - /* - * If we're on native Windows (not Cygwin, which has its own POSIX - * symlinks), but not using the MSVC compiler, then we're using a - * readdir() emulation provided by the MinGW runtime that has no d_type. - * Since the lstat() fallback code reports junction points as directories, - * we need an extra system call to check if we should report them as - * symlinks instead, following our convention. - */ - if (result == PGFILETYPE_DIR && - !look_through_symlinks && - pgwin32_is_junction(path)) - result = PGFILETYPE_LNK; -#endif - return result; } diff --git a/src/include/port.h b/src/include/port.h index d39b04141f..4b978f61bd 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -284,7 +284,6 @@ extern int pgunlink(const char *path); #if defined(WIN32) && !defined(__CYGWIN__) extern int pgsymlink(const char *oldpath, const char *newpath); extern int pgreadlink(const char *path, char *buf, size_t size); -extern bool pgwin32_is_junction(const char *path); #define symlink(oldpath, newpath) pgsymlink(oldpath, newpath) #define readlink(path, buf, size) pgreadlink(path, buf, size) diff --git a/src/include/port/win32_port.h b/src/include/port/win32_port.h index b8cf2e1480..72a0dd572e 100644 --- a/src/include/port/win32_port.h +++ b/src/include/port/win32_port.h @@ -238,7 +238,6 @@ int setitimer(int which, const struct itimerval *value, struct itimerval *oval */ extern int pgsymlink(const char *oldpath, const char *newpath); extern int pgreadlink(const char *path, char *buf, size_t size); -extern bool pgwin32_is_junction(const char *path); #define symlink(oldpath, newpath) pgsymlink(oldpath, newpath) #define readlink(path, buf, size) pgreadlink(path, buf, size) diff --git a/src/port/dirmod.c b/src/port/dirmod.c index ea191e99c6..2818bfd2e9 100644 --- a/src/port/dirmod.c +++ b/src/port/dirmod.c @@ -362,20 +362,4 @@ pgreadlink(const char *path, char *buf, size_t size) return r; } -/* - * Assumes the file exists, so will return false if it doesn't - * (since a nonexistent file is not a junction) - */ -bool -pgwin32_is_junction(const char *path) -{ - DWORD attr = GetFileAttributes(path); - - if (attr == INVALID_FILE_ATTRIBUTES) - { - _dosmaperr(GetLastError()); - return false; - } - return ((attr & FILE_ATTRIBUTE_REPARSE_POINT) == FILE_ATTRIBUTE_REPARSE_POINT); -} #endif /* defined(WIN32) && !defined(__CYGWIN__) */ -- 2.37.1