ReadFile Function doesn't Read Files Correctly - Win32 API

Question

As a part of my Win32 learning, I created a simple program that writes to an existing file named file1.txt and after that, reads from this file to a buffer and displays the buffer on a console. The file's content starts with Hello. The problem will be discussed after the code. This is the code (written in C++/Win32 on Visual Studio 2017):

#include <iostream>
#include <tchar.h>
#include <Windows.h>

#define UNICODE
#define _UNICODE

using namespace std;

int _tmain(int _argc, TCHAR *_argv[])
{
    SECURITY_ATTRIBUTES sa;
    ZeroMemory(&sa, sizeof(sa));
    sa.nLength = sizeof(sa);
    sa.bInheritHandle = false;
    sa.lpSecurityDescriptor = NULL;

    HANDLE hFile = CreateFile(L"file1.txt", GENERIC_READ | GENERIC_WRITE, 0, &sa, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
    LARGE_INTEGER size, physize;
    DWORD fptr = 0;
    BYTE buff[] = " World";

    if (hFile == INVALID_HANDLE_VALUE)
    {
        cout << "Error: " << GetLastError() << "\n";
        return -1;
    }

    cout << "File created!\n";

    GetFileSizeEx(hFile, &size);
    cout << "File Size: " << size.QuadPart << "\n";

    physize.LowPart = GetCompressedFileSize(L"File1.txt", (LPDWORD)&physize.HighPart);
    cout << "Physical Size: " << physize.QuadPart << "\n";

    SetFilePointer(hFile, 0, (PLONG)&fptr, FILE_END);
    WriteFile(hFile, buff, (DWORD)strlen((const char *)buff), &fptr, NULL);

    WaitForSingleObject(hFile, INFINITE);

    SetFilePointer(hFile, 0, (PLONG)&fptr, FILE_BEGIN);
    GetFileSizeEx(hFile, &size);
    cout << "Size now is: " << size.QuadPart << "\n";

    BYTE *buff2 = new BYTE[size.QuadPart + 1];
    buff2[size.QuadPart] = '\0';

    if (!ReadFile(hFile, (LPVOID)buff2, size.QuadPart, &fptr, NULL)) 
    {
        cout << "Error: " << GetLastError() << "\n";
        return -1;
    }

    cout << buff2 << "\n";

    CloseHandle(hFile);
    delete[] buff2, buff;
    return 0;
}

After the writing, the file's content is Hello World. But the output in the console is:

File created!
File Size: 5
Physical Size: 5
Size now is: 11
═══════════

I searched the Web and even this website and didn't find any solution that helped me. In my opinion, the problem is in the reading because the writing has been completed successfully. I even used the function WaitForSignalObject to make sure the writing is ended. Is there something I missed? Thank you for the help.

P.S. My friend has been banned from asking questions in this site, what can he do to remove the ban and continue using the site?

P.S.2 I'm sorry if there is any grammar mistake, English isn't my native language.

Answer 1

The problem is in your misuse of fptr with SetFilePointer().

fptr is set to 0 initially. So, when you call SetFilePointer(hFile, 0, (PLONG)&fptr, FILE_END), you are telling it to seek to offset low:0,high:0 = 0 from the end of the file. So the file pointer gets positioned at offset 5, as expected. SetFilePointer() outputs the new file position to you, so fptr gets updated to the high 32bits of offset 5, which is 0 again.

Then you WriteFile() new data to the file, but you are using fptr to receive the number of bytes actually written. So now fptr is updated to 6 (assuming WriteFile() is successful, which you are not checking for).

Then, you are passing fptr to SetFilePointer(hFile, 0, (PLONG)&fptr, FILE_BEGIN) as the high part. So, instead of telling it to seek to offset low:0,high:0 = 0 from the beginning of the file, you are actually telling it to seek to offset low:0,high=6 = 0x600000000 = 25769803776 from the beginning.

Then you try to read size (11) bytes from offset 25769803776 instead of offset 0, and of course there is no valid data at that high offset, so your buffer gets filled with garbage instead.

The simplest fix is to reset fptr to 0 before seeking to FILE_BEGIN, eg:

fptr = 0; // <-- add this
SetFilePointer(hFile, 0, (PLONG)&fptr, FILE_BEGIN);

Or, you can simply omit the 3rd parameter altogether, since you are not dealing with a large file > 4GB in size:

SetFilePointer(hFile, 0, NULL, FILE_END); // offset 0 from end
...
SetFilePointer(hFile, 0, NULL, FILE_BEGIN); // offset 0 from beginning

If you want to support large files > 4GB properly, don't use separate variables for low and high parts, use (U)LARGE_INTEGER instead (like you already are for GetFileSizeEx() and GetCompressedFileSize()):

#define UNICODE
#define _UNICODE

#include <iostream>
#include <tchar.h>
#include <Windows.h>

using namespace std;

int _tmain(int _argc, TCHAR *_argv[])
{
    SECURITY_ATTRIBUTES sa;
    ZeroMemory(&sa, sizeof(sa));
    sa.nLength = sizeof(sa);
    sa.bInheritHandle = false;
    sa.lpSecurityDescriptor = NULL;

    HANDLE hFile = CreateFile(L"file1.txt", GENERIC_READ | GENERIC_WRITE, 0, &sa, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
    LARGE_INTEGER size, physize, fptr;
    BYTE buff[] = " World";
    DWORD written;

    if (hFile == INVALID_HANDLE_VALUE)
    {
        cout << "Error: " << GetLastError() << "\n";
        return -1;
    }

    cout << "File created!\n";

    GetFileSizeEx(hFile, &size);
    cout << "File Size: " << size.QuadPart << "\n";

    physize.LowPart = GetCompressedFileSize(L"File1.txt", (LPDWORD)&physize.HighPart);
    cout << "Physical Size: " << physize.QuadPart << "\n";

    fptr.QuadPart = 0;
    fptr.LowPart = SetFilePointer(hFile, fptr.LowPart, &fptr.HighPart, FILE_END);
    WriteFile(hFile, buff, (DWORD)strlen((const char *)buff), &written, NULL);

    fptr.QuadPart = 0;
    fptr.LowPart = SetFilePointer(hFile, fptr.LowPart, &fptr.HighPart, FILE_BEGIN);

    GetFileSizeEx(hFile, &size);
    cout << "Size now is: " << size.QuadPart << "\n";

    BYTE *buff2 = new BYTE[size.QuadPart + 1];
    buff2[size.QuadPart] = '\0';

    if (!ReadFile(hFile, (LPVOID)buff2, size.QuadPart, &fptr, NULL)) 
    {
        cout << "Error: " << GetLastError() << "\n";
        return -1;
    }

    cout << buff2 << "\n";

    CloseHandle(hFile);
    delete[] buff2;

    return 0;
}

Or, you can (and should) use SetFilePointerEx() instead:

LARGE_INTEGER ..., fptr;
fptr.QuadPart = 0;

SetFilePointerEx(hFile, fptr, NULL, FILE_END);
...
SetFilePointerEx(hFile, fptr, NULL, FILE_BEGIN);

Answer 2

You are setting your file pointer a bit too far here:

SetFilePointer(hFile, 0, (PLONG)&fptr, FILE_BEGIN);

According to docs for SetFilePointer, the third parameter is:

lpDistanceToMoveHigh

A pointer to the high order 32-bits of the signed 64-bit distance to move. If you do not need the high order 32-bits, this pointer must be set to NULL.

You are setting the move's low order 32-bits to 0, and the high order 32-bits to the value of fptr, which is not 0 when this line is reached. So, you are not moving to offset 0, like you think you are.

Did you mean:

SetFilePointer(hFile, fptr, NULL, FILE_BEGIN);

or:

SetFilePointer(hFile, 0, NULL, FILE_BEGIN);

I strongly suggest to get rid of TCHAR stuff; it has no place in today's world. Just use char and wchar_t explicitly, you should know what do you need.