On Tue, Sep 18, 2018 at 06:04:58PM +0200, Laurenz Albe wrote: > That wouldn't influence pipes, which was what Michael said was a > problem for pg_dump.
Yeah, the authentication blows up badly on that.. You can see all the tests using user and database names with all ASCII range turning red. > I currently have no Windows system close, so I cannot test... I have spent a large portion of my morning trying to test all the solutions proposed, and a winner shows up. Attached are three patches which present all the solutions mentioned, attached here for visibility: - win32-open-michael.patch, or the solution I was working on. This has led me actually nowhere. Even trying to enforce _fmode to happen only on the frontend has caused breakages of pg_dump for authentication. Trying to be smarter than what other binaries do is nice and consistent with the Windows experience I think, still it looks that this can lead to breakages when a utility uses setmode() for some of the output files. I noticed that pgwin32_open missed to enforce the mode used when calling _fdmode, still that solves nothing. - win32-open-tom.patch, which switches pgwin32_fopen() to use text mode all the time if binary is not specified. While this looked promising, I have noticed that this has been causing the same set of errors as what my previous patch has been doing in pg_dump TAP tests. Anyway, a solution needs also to happen for pgwin32_open() as we want direct calls to it to get the right call. - win32-open-laurenz.patch, which enforces to text mode only if binary mode is not defined, which maps strictly to what pre-11 is doing when calling the system _open or _fopen. And surprisingly, this is proving to pass all the tests I ran: bincheck (including pgbench and pg_dump), upgradecheck, recoverycheck, check, etc. initdb --pwfile is not complaining to me either. So the odds are that Laurenz's solution is what we are looking for. Laurenz, Tom, any thoughts to share? I won't back-patch that ;) -- Michael
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index b53d6eb9cc..cb8c7450d9 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -491,15 +491,7 @@ readfile(const char *path)
char *buffer;
int c;
-#ifdef WIN32
- /*
- * On Windows, we have to open the file in text mode so that carriage
- * returns are stripped.
- */
- if ((infile = fopen(path, "rt")) == NULL)
-#else
if ((infile = fopen(path, "r")) == NULL)
-#endif
{
fprintf(stderr, _("%s: could not open file \"%s\" for reading: %s\n"),
progname, path, strerror(errno));
diff --git a/src/port/open.c b/src/port/open.c
index a3ad946a60..9174cdce66 100644
--- a/src/port/open.c
+++ b/src/port/open.c
@@ -122,11 +122,35 @@ pgwin32_open(const char *fileName, int fileFlags,...)
return -1;
}
+ /*
+ * If caller has set neither O_BINARY nor O_TEXT, then look for what
+ * the default mode is for this process, then enforce it. This can
+ * be done only after opening the file and before switching the file
+ * mode.
+ */
+ if ((fileFlags & (O_BINARY | O_TEXT)) == 0)
+ {
+ int pmode = 0;
+
+ /* only MSVC newer than 2015 support _get_fmode */
+#if (_MSC_VER >= 1900)
+ if (_get_fmode(&pmode) < 0)
+ {
+ /* _get_fmode sets errno */
+ CloseHandle(h);
+ return -1;
+ }
+#else
+ pmode = _fmode;
+#endif
+
+ fileFlags |= pmode;
+ }
+
/* _open_osfhandle will, on error, set errno accordingly */
if ((fd = _open_osfhandle((intptr_t) h, fileFlags & O_APPEND)) < 0)
CloseHandle(h); /* will not affect errno */
- else if (fileFlags & (O_TEXT | O_BINARY) &&
- _setmode(fd, fileFlags & (O_TEXT | O_BINARY)) < 0)
+ else if (_setmode(fd, fileFlags & (O_TEXT | O_BINARY)) < 0)
{
_close(fd);
return -1;
@@ -138,6 +162,7 @@ pgwin32_open(const char *fileName, int fileFlags,...)
FILE *
pgwin32_fopen(const char *fileName, const char *mode)
{
+ char fdmode[32];
int openmode = 0;
int fd;
@@ -160,7 +185,36 @@ pgwin32_fopen(const char *fileName, const char *mode)
fd = pgwin32_open(fileName, openmode);
if (fd == -1)
return NULL;
- return _fdopen(fd, mode);
+
+ strcpy(fdmode, mode);
+
+ /*
+ * Like pgwin32_open, look for the default mode to be used for this
+ * file descriptor. Note that it is important to do that again here
+ * for _fdopen below.
+ */
+ if ((openmode & (O_BINARY | O_TEXT)) == 0)
+ {
+ int pmode = 0;
+
+ /* only MSVC newer than 2015 support _get_fmode */
+#if (_MSC_VER >= 1900)
+ if (_get_fmode(&pmode) < 0)
+ {
+ /* get_fmode sets errno */
+ _close(fd);
+ return NULL;
+ }
+#else
+ pmode = _fmode;
+#endif
+
+ snprintf(fdmode, sizeof(fdmode), "%s%s%s", fdmode,
+ (pmode & O_BINARY) != 0 ? "b" : "",
+ (pmode & O_TEXT) != 0 ? "t" : "");
+ }
+
+ return _fdopen(fd, fdmode);
}
#endif
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index b53d6eb9cc..cb8c7450d9 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -491,15 +491,7 @@ readfile(const char *path)
char *buffer;
int c;
-#ifdef WIN32
- /*
- * On Windows, we have to open the file in text mode so that carriage
- * returns are stripped.
- */
- if ((infile = fopen(path, "rt")) == NULL)
-#else
if ((infile = fopen(path, "r")) == NULL)
-#endif
{
fprintf(stderr, _("%s: could not open file \"%s\" for reading: %s\n"),
progname, path, strerror(errno));
diff --git a/src/port/open.c b/src/port/open.c
index a3ad946a60..63a7a1d2fb 100644
--- a/src/port/open.c
+++ b/src/port/open.c
@@ -71,6 +71,19 @@ pgwin32_open(const char *fileName, int fileFlags,...)
_O_SHORT_LIVED | O_DSYNC | O_DIRECT |
(O_CREAT | O_TRUNC | O_EXCL) | (O_TEXT | O_BINARY))) == fileFlags);
+#ifdef FRONTEND
+ /*
+ * Since PostgreSQL 12, those concurrent-safe versions of open() and
+ * fopen() can be used by frontends, having as side-effect to switch
+ * the file-transaction mode from O_TEXT to O_BINARY if none is
+ * specified. Caller may want to enforce the binary or text mode,
+ * but if nothing is defined make sure that the default mode maps
+ * with what versions older than 12 have been doing.
+ */
+ if ((fileFlags & O_BINARY) == 0)
+ fileFlags |= O_TEXT;
+#endif
+
sa.nLength = sizeof(sa);
sa.bInheritHandle = TRUE;
sa.lpSecurityDescriptor = NULL;
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index b53d6eb9cc..cb8c7450d9 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -491,15 +491,7 @@ readfile(const char *path)
char *buffer;
int c;
-#ifdef WIN32
- /*
- * On Windows, we have to open the file in text mode so that carriage
- * returns are stripped.
- */
- if ((infile = fopen(path, "rt")) == NULL)
-#else
if ((infile = fopen(path, "r")) == NULL)
-#endif
{
fprintf(stderr, _("%s: could not open file \"%s\" for reading: %s\n"),
progname, path, strerror(errno));
diff --git a/src/port/open.c b/src/port/open.c
index a3ad946a60..85ab06e518 100644
--- a/src/port/open.c
+++ b/src/port/open.c
@@ -154,7 +154,7 @@ pgwin32_fopen(const char *fileName, const char *mode)
if (strchr(mode, 'b'))
openmode |= O_BINARY;
- if (strchr(mode, 't'))
+ else
openmode |= O_TEXT;
fd = pgwin32_open(fileName, openmode);
signature.asc
Description: PGP signature
