• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 284
  • Last Modified:

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
            delete [] pch;
            return true;
            delete [] pch;
            return false;
1 Solution
You need to allocate one more character than you want to read.  EG,

char * pch = new char[nLength + 1];
pch[nLength + 1] = NULL;

I prefer to clear out the whole buffer that was just allocated.  Something like this:

memset(pch, 0, nLength + 1);

this sets the whole buffer to nulls.  That way, if you only read in 5 bytes and your length is 100, you are certain that your string ends with a NULL terminator.

Your nBytesRead == nLength condition ensures that you have read in exactly as many characters as you allocated, thus you don't really know if your buffer ends with a NULL.
 char *pch = new char[nLength]
allocates nLength characters, usable from
  pch[nLength - 1]

  pch[nLength] = 0;

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


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.
awdAuthor Commented:
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.
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.
Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.

Join & Write a Comment

Featured Post

The new generation of project management tools

With monday.com’s project management tool, you can see what everyone on your team is working in a single glance. Its intuitive dashboards are customizable, so you can create systems that work for you.

Tackle projects and never again get stuck behind a technical roadblock.
Join Now