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

The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation: not tested

I tested the patch at hand, and it performs as expected. Files larger than 4GB can be imported.

Steps:
0) create a csv-file that is sufficiently big (>4GB), and one that is small. Use these files to test.
1a) Attempt to import the small file using devel-version.
1b) EXPECTED: success, ACTUAL: success
2a) Attempt to import the big file using devel-version.
2b) EXPECTED: failure, ACTUAL: failure
3) Apply patch and build new version
4a) Attempt to import the small file using patched-version.
4b) EXPECTED: success, ACTUAL: success
4a) Attempt to import the big file using patched-version.
4b) EXPECTED: success, ACTUAL: success

The code looks sensible, it is easy to read and follow. The code uses appropriate win32 functions to perform the task.

Code calculates file size using the following method: buf->st_size = ((__int64) fiData.nFileSizeHigh) << 32 | (__int64)(fiData.nFileSizeLow);
The hard coded constant 32 is fine, nFileSizeHigh is defined as a DWORD in the Win32 API, which is a 32 bit unsigned integer. There is no need to a dynamic calculation.

There are minor "nit-picks" that I would change if it were my code, but do not change the functionality of the code.

1)
if (GetFileAttributes(name) == INVALID_FILE_ATTRIBUTES)
{
errno = ENOENT;
return -1;
}

Here I would call _dosmaperr(GetLastError()) instead, just to take account of the possibility that some other error occurred. Following this change there are slight inconsistency in the order of "CloseHandle(hFile), errno = ENOENT; return -1" and "_dosmaperr(GetLastError()); CloseHandle(hFile); return -1". I would prefer consistent ordering, but that is not important.

The new status of this patch is: Ready for Committer