From ea47a8af0332876629b53620788d40bcb7b1e96c Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Tue, 26 Jul 2022 12:56:06 +1200 Subject: [PATCH 2/2] WIP Do not pollute Cygwin namespace with Windows headers. Establish that __CYGWIN__ and WIN32 should generally not be defined at the same time. If ever it's necessary to access Windows APIs directly when building for Cygwin, that should be done in an extremely localized way, not allowing Windows macros and declarations to leak into other translation units. dirmod.c initially looked like a potential case for a localized exemption, but on closer inspection it doesn't currently have any reason to include -- so don't. In passing, remove anachronistic comments about ancient Windows versions. --- src/include/pg_config_manual.h | 2 +- src/include/port.h | 17 +++++++------ src/port/dirmod.c | 46 +++++++++++++++++----------------- 3 files changed, 33 insertions(+), 32 deletions(-) diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h index 5ee2c46267..0d1d5d7c9b 100644 --- a/src/include/pg_config_manual.h +++ b/src/include/pg_config_manual.h @@ -148,7 +148,7 @@ * fork()). On other platforms, it's only useful for verifying those * otherwise Windows-specific code paths. */ -#if defined(WIN32) && !defined(__CYGWIN__) +#if defined(WIN32) #define EXEC_BACKEND #endif diff --git a/src/include/port.h b/src/include/port.h index d39b04141f..8cf1112a54 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -18,10 +18,12 @@ /* * Windows has enough specialized port stuff that we push most of it off * into another file. - * Note: Some CYGWIN includes might #define WIN32. */ -#if defined(WIN32) && !defined(__CYGWIN__) +#if defined(WIN32) #include "port/win32_port.h" +#if defined(__CYGWIN__) +#error "__CYGWIN__ should not be defined at the same time as WIN32" +#endif #endif /* socket has a different definition on WIN32 */ @@ -150,7 +152,7 @@ extern int pg_disable_aslr(void); #define EXE "" #endif -#if defined(WIN32) && !defined(__CYGWIN__) +#if defined(WIN32) #define DEVNULL "nul" #else #define DEVNULL "/dev/null" @@ -276,12 +278,11 @@ extern int pgunlink(const char *path); * Win32 also doesn't have symlinks, but we can emulate them with * junction points on newer Win32 versions. * - * Cygwin has its own symlinks which work on Win95/98/ME where - * junction points don't, so use those instead. We have no way of - * knowing what type of system Cygwin binaries will be run on. - * Note: Some CYGWIN includes might #define WIN32. + * Cygwin has its own symlinks that work where junction points don't, so use + * those instead. We have no way of knowing what type of system Cygwin + * binaries will be run on. */ -#if defined(WIN32) && !defined(__CYGWIN__) +#if defined(WIN32) 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); diff --git a/src/port/dirmod.c b/src/port/dirmod.c index 7ce042e75d..1bfbead098 100644 --- a/src/port/dirmod.c +++ b/src/port/dirmod.c @@ -6,8 +6,10 @@ * Portions Copyright (c) 1996-2022, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * This includes replacement versions of functions that work on - * Win32 (NT4 and newer). + * This includes replacement versions of functions that work on Windows. + * For Cygwin, the purpose of these replacements is to provide retry loops + * around POSIX functions. For native Windows, we also redirect to + * native Windows APIs. * * IDENTIFICATION * src/port/dirmod.c @@ -21,25 +23,24 @@ #include "postgres_fe.h" #endif +#if defined(WIN32) && defined(__CYGWIN__) +#error "WIN32 should not be defined at the same time as __CYGWIN__" +#endif + +#if !defined(WIN32) && !defined(__CYGWIN__) +#error "one of WIN32 or __CYGWIN__ is expected" +#endif + /* Don't modify declarations in system headers */ -#if defined(WIN32) || defined(__CYGWIN__) #undef rename #undef unlink -#endif #include #include -#if defined(WIN32) || defined(__CYGWIN__) -#ifndef __CYGWIN__ +#if defined(WIN32) #include -#else -#include -#include #endif -#endif - -#if defined(WIN32) || defined(__CYGWIN__) /* * pgrename @@ -56,24 +57,24 @@ pgrename(const char *from, const char *to) * someone else to close the file, as the caller might be holding locks * and blocking other backends. */ -#if defined(WIN32) && !defined(__CYGWIN__) +#if defined(WIN32) while (!MoveFileEx(from, to, MOVEFILE_REPLACE_EXISTING)) #else while (rename(from, to) < 0) #endif { -#if defined(WIN32) && !defined(__CYGWIN__) +#if defined(WIN32) DWORD err = GetLastError(); _dosmaperr(err); /* - * Modern NT-based Windows versions return ERROR_SHARING_VIOLATION if - * another process has the file open without FILE_SHARE_DELETE. - * ERROR_LOCK_VIOLATION has also been seen with some anti-virus - * software. This used to check for just ERROR_ACCESS_DENIED, so - * presumably you can get that too with some OS versions. We don't - * expect real permission errors where we currently use rename(). + * Windows returns ERROR_SHARING_VIOLATION if another process has the + * file open without FILE_SHARE_DELETE. ERROR_LOCK_VIOLATION has also + * been seen with some anti-virus software. This used to check for + * just ERROR_ACCESS_DENIED, so presumably you can get that too with + * some OS versions. We don't expect real permission errors where we + * currently use rename(). */ if (err != ERROR_ACCESS_DENIED && err != ERROR_SHARING_VIOLATION && @@ -121,10 +122,9 @@ pgunlink(const char *path) /* We undefined these above; now redefine for possible use below */ #define rename(from, to) pgrename(from, to) #define unlink(path) pgunlink(path) -#endif /* defined(WIN32) || defined(__CYGWIN__) */ -#if defined(WIN32) && !defined(__CYGWIN__) /* Cygwin has its own symlinks */ +#if defined(WIN32) /* * pgsymlink support: @@ -352,4 +352,4 @@ pgwin32_is_junction(const char *path) } return ((attr & FILE_ATTRIBUTE_REPARSE_POINT) == FILE_ATTRIBUTE_REPARSE_POINT); } -#endif /* defined(WIN32) && !defined(__CYGWIN__) */ +#endif /* defined(WIN32) */ -- 2.35.1