Date: Tue, 18 Sep 2018 11:42:27 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: pgsql: Allow concurrent-safe open() and fopen() in frontend code
 for Wi
Message-ID: <20180918024227.GI31460@paquier.xyz>
References: <23543.1536938575@sss.pgh.pa.us>
 <20180914224939.GA1178@paquier.xyz>
 <20180917114113.GA31460@paquier.xyz>
 <17194.1537191697@sss.pgh.pa.us>
 <20180917140202.GF31460@paquier.xyz>
 <19655.1537195712@sss.pgh.pa.us>
 <f62917d5-cc77-c6ac-847f-44f922ccdd97@2ndQuadrant.com>
 <3883.1537200836@sss.pgh.pa.us>
 <11148.1537227504@sss.pgh.pa.us>
 <20180918001143.GG31460@paquier.xyz>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="kr14OxHsRwZHHqxS"
Content-Disposition: inline
In-Reply-To: <20180918001143.GG31460@paquier.xyz>
Precedence: bulk


--kr14OxHsRwZHHqxS
Content-Type: multipart/mixed; boundary="K1SnTjlYS/YgcDEx"
Content-Disposition: inline


--K1SnTjlYS/YgcDEx
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Tue, Sep 18, 2018 at 09:11:43AM +0900, Michael Paquier wrote:
> What I think I broke is that CreateFile ignores what _fmode uses, which
> has caused the breakage, while calling directly open() or fopen() does
> the work.  There are also other things assuming that binary mode is
> used, you can grep for "setmode" and see how miscinit.c or pg_basebackup
> do the job.

I have been playing with this stuff, and hacked the attached.  Now, while
TAP tests of initdb and pgbench are happy (I can actually see the past
failure as well), pg_dump complains at authentication time when using
plain-text mode when using databases with all ASCII characters.  That's
not something I expected first, but _get_fmode also influences
operations like pipe(), which is something that pg_dump uses, and
setmode is enforced to binary mode only when adapted.

I am getting to wonder if what's present on HEAD represents actually the
best deal we could get.  Attached is the patch I used for reference.
Thoughts?
--
Michael

--K1SnTjlYS/YgcDEx
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="win32-open-mode-v1.patch"
Content-Transfer-Encoding: quoted-printable

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;
=20
-#ifdef WIN32
-	/*
-	 * On Windows, we have to open the file in text mode so that carriage
-	 * returns are stripped.
-	 */
-	if ((infile =3D fopen(path, "rt")) =3D=3D NULL)
-#else
 	if ((infile =3D fopen(path, "r")) =3D=3D 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..f7a296ef28 100644
--- a/src/port/open.c
+++ b/src/port/open.c
@@ -71,6 +71,28 @@ pgwin32_open(const char *fileName, int fileFlags,...)
 						 _O_SHORT_LIVED | O_DSYNC | O_DIRECT |
 						 (O_CREAT | O_TRUNC | O_EXCL) | (O_TEXT | O_BINARY))) =3D=3D fileFla=
gs);
=20
+	/*
+	 * If caller has set neither O_BINARY nor O_TEXT, then look for what
+	 * the default mode is for this process, then enforce it.
+	 */
+	if ((fileFlags & (O_BINARY | O_TEXT)) =3D=3D 0)
+	{
+		int		pmode =3D 0;
+
+		/* only MSVC newer than 2015 support _get_fmode */
+#if (_MSC_VER >=3D 1900)
+		if (_get_fmode(&pmode) < 0)
+		{
+			/* get_fmode sets errno */
+			return -1;
+		}
+#else
+		pmode =3D _fmode;
+#endif
+
+		fileFlags |=3D pmode;
+	}
+
 	sa.nLength =3D sizeof(sa);
 	sa.bInheritHandle =3D TRUE;
 	sa.lpSecurityDescriptor =3D NULL;

--K1SnTjlYS/YgcDEx--

--kr14OxHsRwZHHqxS
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAlugZhMACgkQnvQgOdby
QH3O9hAAkthxqlGm0I6YE6JEtsH6nq36M3E2zKYcMrmV0WSV/guOM4c5aBKYUbfm
b0qIDmAbb++ocifpAUOSD0MX8alEgUMgZ+QOhlDnjEF2LBwlzM9jBO1PvPkfJj8w
Vn2QdeSRsA8azNX1M39w1EXMQvAv1V8c/fMWLzvT/bBaL8ppjmnEZe9iGD/okv1G
GPjejAEP4rcyjIhPDs2fhG9SfmgU31UbkvsnvomEuNQIo2m/oScDTspxuXz0Wk8R
j6L23FdJczA0/kjeSO4/4av2QRou72Lsd36Q/LtCZW5I7WN4nUcO+VHhHTnIx/qH
BSRN0I1bYcgMGde2s63KsFBiTZnd2QfTkHPKSz7W/6ntPMr1RMrnX01KBK5ipB+h
Ys3CdwOS9Sj5bTzgKx5FyUn+GXxKi1PirkXiZN+1maGxN+47V7l3pk74udOxiHMi
oSLDy2aNnOEmAhXKRzWmmCaFgRFoOSruQ1w7R2YeymbW4zpem+jwiXSU7AF0xW1D
kczfx70BGv4jatGmKPsoQo9gv56HFdA3pDp0Xk25U2s3AWE5rn9cwBau3VNGSCeR
E+uPT/0KoXyGLCk1rAyPtJPTKvkYkkaWXD+PVwr82HH3NKTAr9+6bs989S+rzDua
5yT6WgJ2Pv1EzWUb6lvpnPUmKGlM1O2qD4yrBU0v8GYiqS/cjFk=
=hopf
-----END PGP SIGNATURE-----

--kr14OxHsRwZHHqxS--