Re: BUG #15858: could not stat file - over 4GB

On Wed, Jun 26, 2019 at 4:23 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> 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:

I wanted to make a quick test on the previous patch. So let me state
what have I tested and what I have not: it builds and pass tests in
Windows and Cygwin, but I have not setup a MinGW environment.

> 1) Are we able to fix the issues with stat() calls on files larger
> than 2GB and report a correct size?

I have successfuly tested a COPY with large files.

> 2) Are we able to detect properly that files pending for deletion are
> discarded with ENOENT?

Cannot reproduce reliably, but is using the same logic as pgwin32_safestat().

> 3) Are frontends able to use the new layer?

After removing UNSAFE_STAT_OK, is this still an issue?

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

The changes in configuration are meant for gcc compilations in Windows
(Cygwin and Mingw).

> 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.

Ok.

> 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.

I have only found something similar in ./src/port/gettimeofday.c, but
not sure if this patch should touch that code.

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

I wanted to keep as much of the original code as possible, but if this
is found as a viable solution, what shape should it have?

> 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.

The previous patch was broken, taken from the wrong local branch
(sorry about that). The attached is still a WIP but it has to do the
things above-mentioned.

Regards,

Juan José Santamaría Flecha