File read write using MFC CFile

Question

I'm wondering why the below code not writing correct data to file. If I change the buffer size to some greater amout this code works fine.

For the below code, if I try to read a file less than 500 bytes it works good, but for a bigger file simply I have to increase the buffer. What I'm missing in the reading loop?

  const int iBuffSiz = 500;
  char chBuffer[iBuffSiz];
  memset(chBuffer, 0, sizeof(chBuffer));
    CFile file;
  CFile fileO;

  if(file.Open(XML_FILE_NAME, CFile::modeRead | CFile::typeBinary) == FALSE)
  {
    return;
  }

  if(fileO.Open(XML_FILE_NAME_O, CFile::modeCreate | CFile::modeWrite | CFile::typeBinary) == FALSE)
  {
    return;
  }

  while(file.Read(chBuffer, iBuffSiz) > 0)
  {
    try{
      UINT iCount = strlen(chBuffer);
      fileO.Write(chBuffer, iCount);
    }
    catch (CFileException *exp)
    {
      TCHAR szCause[255];
      exp->GetErrorMessage(szCause, 255);
    }
  }

  //Closing file handle and socket after complete file send
  file.Close();
  fileO.Close();

You are opening the input file as _CFile::typeBinary_, but use _strlen()_ to get the length of whatever. — plaintext, Mar 17 '15 at 19:19
That's slightly misleading, @plaintext, as the `CFile::typeBinary` has nothing to do with the issues. The problem is rather that the OP has no guarantee, with or without that flag, that the buffer contents are a NUL-terminated string, which is required in order to use `strlen()`. Instead, check the documentation of CFile, I think it can tell you how many bytes it read. Lastly, two more hints: There are better ways to copy a file (search the web!) and catching and then completely ignoring exceptions is actively harmful. — Ulrich Eckhardt, Mar 17 '15 at 20:16

Barmak Shemirani · Accepted Answer · 2015-03-18T10:53:45.917

3

CFile::Read doesn't null terminate the buffer, so strlen of buffer can be larger than the buffer, it runs in to buffer overflow. There is a way to get around this, but strlen still won't work with binary files, this time the buffer is cut too short. So it's better to use the value returned by CFile::Read

UINT iCount;
while( ( iCount = file.Read(chBuffer, iBuffSiz) ) > 0 )
{
   try
   {
      fileO.Write(chBuffer, iCount);
   }
   catch (CFileException *exp)
   {
      TCHAR szCause[255];
      exp->GetErrorMessage(szCause, 255);
   }
}

edited Mar 18 '15 at 10:53

answered Mar 18 '15 at 05:48

Barmak Shemirani

30,904
6
40
77

This works perfect and the reason is obvious. Thanks. Just one thing that I'm curious to know. If I change the while loop to 'while( iCount = file.Read(chBuffer, iBuffSiz) > 0)' why it can't able to write the proper data into output file. >0 suppose to do the same work as above code, while loop will run till the value is true. Whats the issue here. – hypheni Mar 18 '15 at 09:02
It needs another set of parenthesis to carry the result of `=` operator. I edited the answer with `> 0`. I am pretty sure CFile::Read doesn't return negative numbers but better be safe. Also it was confusing before. – Barmak Shemirani Mar 18 '15 at 10:54

score 0 · Answer 2 · answered Mar 18 '15 at 14:10

Agree with Barmark that CFile::Read may return count more than buffer size specified

ONe more point I have is

*** You need to clear buffer after writing it to another file as it is pointing to string you already written to destination file , and because of this you may not get exact replica of file that is desired.

  while(file.Read(chBuffer, iBuffSiz-1) > 0)
  {
    try{
      UINT iCount = strlen(chBuffer);
      fileO.Write(chBuffer, iCount);
      memset(chBuffer, 0, sizeof(chBuffer));
    }
    catch (CFileException *exp)
    {
      TCHAR szCause[255];
      exp->GetErrorMessage(szCause, 255);
    }
  }

CFile::Read will return at maximum the number of bytes you specified as its second parameters, its the *strlen* on a not null terminated buffer that leads to undefined behaviour. Also, you don't need to clear the buffer if you use the CFile::Read return value to determine how much buffer you can use and how much has to be discarded. — Fabio C., Nov 12 '18 at 14:38

File read write using MFC CFile

2 Answers2