Date: Wed, 26 Jun 2019 11:22:36 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Juan =?iso-8859-1?Q?Jos=E9_Santamar=EDa?= Flecha
 <juanjo.santamaria@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, williamedwinallen@live.com,
	pgsql-bugs@lists.postgresql.org,
	Magnus Hagander <magnus@hagander.net>
Subject: Re: BUG #15858: could not stat file - over 4GB
Message-ID: <20190626022236.GE1714@paquier.xyz>
References: <15858-9572469fd3b73263@postgresql.org>
 <20190619012604.GC2135@paquier.xyz>
 <CAC+AXB0j9pN4z5XxMbn6+kcE_QM+CErjfYp-vVz2rU4suU7u=g@mail.gmail.com>
 <16138.1560966010@sss.pgh.pa.us>
 <17044.1560967356@sss.pgh.pa.us>
 <CAC+AXB2Fu5oJm7pmS1Pg3Gf_XyxSfun1DA7dPf+yZFQ3p5vV3g@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="HCdXmnRlPgeNBad2"
Content-Disposition: inline
In-Reply-To: 
 <CAC+AXB2Fu5oJm7pmS1Pg3Gf_XyxSfun1DA7dPf+yZFQ3p5vV3g@mail.gmail.com>
Precedence: bulk


--HCdXmnRlPgeNBad2
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Jun 25, 2019 at 12:00:45PM +0200, Juan Jos=E9 Santamar=EDa Flecha w=
rote:
> I will not have much time for this list in the next couple of weeks,
> so I will send this patch in its current WIP state rather than
> stalling without a reply.
>=20
> Most of its functionality comes from Sergey's patch with some cosmetic
> changes, and the addition of the 64 bits struct stat and fstat().

The former patch was rather impressive.  Or scary. Or both.  At which
extent have you tested it?  I think that we really need to make sure
of a couple of things which satisfy our needs:
1) Are we able to fix the issues with stat() calls on files larger
than 2GB and report a correct size?
2) Are we able to detect properly that files pending for deletion are
discarded with ENOENT?
3) Are frontends able to use the new layer?

It seems to me that you don't need the configure changes.

Instead of stat_pg_fixed which is confusing because it only involves
Windows, I would rename the new file to stat.c or win32_stat.c.  The
location in src/port/ is adapted.  I would also move out of
win32_port.h the various inline declarations and keep only raw
declarations.  That could be much cleaner.

The code desperately needs more comments to help understand its
logic.  Don't we have in the tree an equivalent of cvt_ft2ut?  What
does cvt_attr2uxmode do?  It would be nice to avoid conversion
wrappers as much as possible, and find out system-related equivalents
if any, and actually if necessary.

+static unsigned short
+cvt_attr2uxmode(int attr, const _TCHAR * name)
This looks rather bug-prone...

I think that this stuff has not been tested and would break at
compilation.  If src/tools/msvc/Mkvcbuild.pm is not changed, then the
new file won't get included in the compiled set.=20
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAl0S1uwACgkQnvQgOdby
QH0TGw/+MDHr34ADCXIBed+mjvcb5U5UF6hAxeg9CTXcbFXuSAnLrjgIP2dvV0xX
kVvWcxehukbpRv4DQEGJ07cP13PhD2IMZJfE/cRS0iyVaSl6Vjk2PpmOPQRMMfgn
cOGO/Uf3t85BM5+Ns9/MjprUfSJVZ2/gh3ZCkQMFgcd0FMwD6ICCQz90UHUHD0nt
l9Z8K0G5h9lr1pIVPqF5fAUgdXFFNQPsZV3Ck8ND8LOv9tUpvnclB9SNwiIPzOLP
D7O4Bt8QzF3eIadI4g2JsTuMny6ydqGKhAMhnqUmxAIq2oMPYYtPIj1GMS/S4g0c
cAblnnw/PjW1+qfbOgVkrNNjgQzGtuJ9Cw8Dn/zuI/y+AiXvyAamhPnh3LVe1nXz
8F0xvJk3y3z9k5xHVktzoR9w1Xq0u/b9+knQMmCU6ZtGavpNHJbDFruoetyHn7/+
j9WHvjXmFDYuQPuSpOdA6H/GmhmIBUc6chMqswvy5EyktkVd7dZ5/6EKMqa4yoPs
hEv/5J2bGfnXJ5Rx4RvQRB6b4ZRO/v5AiyRYy+jfHBXqj9Dg0WileP+rOOD/rD9a
ECACtxVt+12bjjK3tpFqtZoZ+h3PfQ9sF4MdDK8CNsN3Qb2SABM3Xpg0kzk6Rqh5
omF97sODessO4VTRk11YBDu7ZS1jwZ8EOtAAUUZM5OtkP5WTrPo=
=ETwI
-----END PGP SIGNATURE-----

--HCdXmnRlPgeNBad2--