-3

Trying to create a handle from existing file (test.txt), read the data to buffer and printing the buffer using printf.

Everything apparently works fine - There is no errors and the number of bytes read by ReadFile is correct. But when I printing the buffer with printf, some garbage prints after the data from the txt file to the screen:

enter image description here

The code is:

#include <windows.h>
#include <stdio.h>

WCHAR input_file[9] = L"test.txt";
LPCWSTR p_input_file = input_file;

#define BUFFER_SIZE 24

int main()
{
    HANDLE hFile = NULL;
    CHAR inBuffer[BUFFER_SIZE];
    LPVOID pbuffer = inBuffer;
    DWORD nNumberOfBytesToRead = BUFFER_SIZE;
    DWORD nNumberOfBytesRead;

    hFile = CreateFile(input_file,               
        GENERIC_READ,          
        FILE_SHARE_READ,       
        NULL,                  
        OPEN_EXISTING,         
        FILE_ATTRIBUTE_NORMAL, 
        NULL);                 

    BOOL reading_result = ReadFile(hFile,
        pbuffer,
        nNumberOfBytesToRead,
        &nNumberOfBytesRead,
        NULL);

    int error_code_value = GetLastError();

    if (error_code_value == 0)
    {
        printf("Work properly. The text is: %s \n", inBuffer);
    }
    else if (error_code_value == ERROR_INVALID_HANDLE)
    {
        printf("Handle is loaded in invalid way.\n");
    }
    else
    {
        printf("Worked wrong. error code: %d. \n", GetLastError());
    }


    CloseHandle(hFile);
}
genpfault
  • 51,148
  • 11
  • 85
  • 139
TomerY
  • 1
  • 3
  • 5
    The `%s` format specifier expects a nul-terminated string. – Mark Benningfield Apr 28 '23 at 16:12
  • 2
    The `ReadFile` function only reads a sequence of bytes. It doesn't know what those bytes represents, and it doesn't care. That means if you want to use the data as a null-terminated string then you need to make sure to format the data that way. By adding your own explicit null-terminator. – Some programmer dude Apr 28 '23 at 16:14
  • 2
    Alternatively, you can tell `printf()` how many characters to print, then you don't need to null-terminate the buffer, eg: `printf("Work properly. The text is: %.*s \n", (int)nNumberOfBytesRead, inBuffer);`. However, since you tagged the question as [tag:c++], consider using a pure C++ solution via `std::string`, see [How do I read an entire file into a std::string in C++?](https://stackoverflow.com/questions/116038/). – Remy Lebeau Apr 28 '23 at 16:31
  • 1
    On a side note, your error handling is wrong. `if (error_code_value == 0)` should be `if (reading_result != 0)` instead (or simply `if (reading_result)`). Don't look at `GetLastError()` unless you first validate that an API actually failed to begin with. Do the same with `CreateFile()`, check if it returned `INVALID_HANDLE_VALUE` (not `NULL`) *before* you then call `ReadFile()`. This is especially important with `CreateFile()` as `GetLastError()` can return *non-zero* error codes when `CreateFile()` is *successful*. – Remy Lebeau Apr 28 '23 at 16:34
  • If this was supposed to be a C question, why did you tag C++ instead?! I don't see anything C++specific in your snippet. – Red.Wave Apr 28 '23 at 19:09
  • `#define BUFFER_SIZE 24` - why are you using a preprocessor macro when you could use a proper `const` or `constexpr` variable? And why the global variables? – Jesper Juhl May 01 '23 at 10:47

2 Answers2

1

printf with %s prints characters starting at the address you say until it finds a null character.

ReadFile doesn't put a null character in your array (why would it?) and nor do you, so there isn't one.

You know how many bytes were read because it's in the variable nNumberOfBytesRead. You can choose to print that many bytes by using %.*s and also telling printf the number of bytes, or you can choose to store a null character in the array after all the ones that you read. In the second case you must make sure that the array has space for the read characters and also the null character. If the array has 24 characters then you can only read 23 and then make the last one null.

user253751
  • 57,427
  • 7
  • 48
  • 90
0

You want this (explanations in the comments):

...
int main()
{
  HANDLE hFile = NULL;
  CHAR inBuffer[BUFFER_SIZE + 1];  // + 1 for the null character
  LPVOID pbuffer = inBuffer;
  DWORD nNumberOfBytesToRead = BUFFER_SIZE;
  DWORD nNumberOfBytesRead;

  hFile = CreateFile(input_file,
    GENERIC_READ,
    FILE_SHARE_READ,
    NULL,
    OPEN_EXISTING,
    FILE_ATTRIBUTE_NORMAL,
    NULL);

  if (hFile == INVALID_HANDLE_VALUE)  // first thing to do
  {
    printf("CreateFile failed: error code = %d.\n", GetLastError());
  }
  else
  {
    BOOL reading_result = ReadFile(hFile,
      pbuffer,
      nNumberOfBytesToRead,
      &nNumberOfBytesRead,
      NULL);

    if (reading_result)  // you need to check reading_result
    {
      ((char*)pbuffer)[nNumberOfBytesRead] = 0;   // put a null terminator
      printf("Works properly. The text is: %s \n", inBuffer);
    }
    else
    {
      printf("Worked wrong. error code: %d.\n", GetLastError());
    }

    CloseHandle(hFile);
  }
}
Jabberwocky
  • 48,281
  • 17
  • 65
  • 115