asked on

Dynamic char* Allocation

What is wrong with the following code? It crashes on the delete statement (true condition) every time I run it.

bool ReadWriteString(CFile* fileIn, CStdioFile* fileOut, CString strOut, unsigned short nLength)
{
      UINT nBytesRead;
      char *pch = new char[nLength];

      pch[nLength] = NULL;

      nBytesRead = fileIn->Read(pch, nLength);

      if (nBytesRead == nLength)
      {
            // Write Label
            fileOut->WriteString(strOut + "\n");
            // Write value
            fileOut->WriteString(pch);
            fileOut->WriteString("\n");
            delete [] pch;
            return true;
      }
      else
      {
            delete [] pch;
            return false;
      }
}

ASKER CERTIFIED SOLUTION

mandhjo

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

KangaRoo

char *pch = new char[nLength]
allocates nLength characters, usable from
pch[0]
to
pch[nLength - 1]

so
pch[nLength] = 0;

will write in un allocated memory

nietod

Except the code doesn't try to write past the end of the allocated array. The

fileOut->WriteString(pch)

might die because the string might not be terminated (can't tell this from the shown code.) However there is no reason why the delete [] would die.

awd

ASKER

mandhjo was correct. Although I think he made a careless mistake. The line:
pch[nLength + 1] = NULL should read
pch[nLength] = NULL after making the suggested change.
Thanks for the comments everyone.

mandhjo

As I mentioned later in my answer, I believe the correct way to initialize any buffer is to initialize ALL of it to NULL, not just what you THINK to be the last character.

Thanks for the points, glad I could help.