0

I am getting heap corruption error while trying to free memory with delete

Here's code

char** split(char* inputstr, char delim, int& count){

char** ostr=NULL;
int numStr = 0;
int i=0,j,index=0;


while(inputstr[i]){ 

    if(inputstr[i++]==delim)
        numStr++;

}

if(inputstr[i-1]!=delim)
    numStr++;


count= numStr;
ostr = new char*[numStr];

i=0;
while(inputstr[i])
{
    j=i;
    while(inputstr[j] && inputstr[j] != delim)
        j++;

    ostr[index] = new char[j-i+1];

    //istr[j] = 0;

    strncpy(ostr[index], inputstr+i,j-i);

    ostr[index++][j-i]=0;

    i=j+1;

}

return ostr;

}

for(int i=0,countStr;i<_numComp;i++){

            char** _str = split(str[1+i],':',countStr);

            message.lastTransList.cmpName[i] = new char[strlen(_str[0])+1];
            strcpy(message.lastTransList.cmpName[i],_str[0]);
            message.lastTransList.price[i] = atof(_str[1]);

            for(int i=0; i<countStr;i++)
            {
                delete[] _str[i];    //this is working fine
                _str[i] = 0;

            }

            delete[] _str;     //exception is thrown at this line
        }

I am not able to find the problem. Please help !

5
  • 2
    Do you have a reason to use c-strings and raw arrays? std::string and std::vector would probably help you a lot here
    – carlpett
    Aug 31, 2011 at 7:52
  • You can indicate what the error exact is, what you have tried until now, and try to revert the code to a minimal failing (but compiling) example.
    – KillianDS
    Aug 31, 2011 at 8:24
  • Did you consider running this program with valgrind?
    – vine'th
    Aug 31, 2011 at 8:50
  • I am using raw arrays because I cant use vectors in this project. The exact error is heap corruption, I tried debugging using breakpoints, The error is appearing on line delete[] _str, rest all is working correctly. No, I dont know nothing about Valgrind. Should I use it. How it will help?
    – Sumit Jain
    Aug 31, 2011 at 9:07
  • Valgrind will help pinpoint the source of your heap corruption (down to the line number in most cases.) It's the perfect tool for this use case if you're on a platform where it's supported.
    – brandx
    Aug 31, 2011 at 11:56

1 Answer 1

0

It's hard to see any error, there could be something wrong with your indexing that's causing a buffer overrun in the split function that's caught only when you try to delete the char** array.

How about converting to std::string and std::vectors like carlpett recommends (it's a good recommendation).

something like this:

void split(const std::string& str_, char delimiter_, std::vector<std::string>& result_)
{
  std::string token;
  std::stringstream stream(str_);
  while( std::getline(stream, token, delimiter_) ) result_.push_back(token);
}

Then, you just call it with your string, delimiter and an empty std::vector and end up with a populated vector of substrings. You don't have to use new/delete and worry about the memory issues.

5
  • Why do you pass result as an argument instead of returning it? Even better, recommend boost, that has split on token built in.
    – KillianDS
    Aug 31, 2011 at 8:27
  • We are returning the split-ted string as 2D character array and passing the number of strings as an argumnet by reference as we cant return two things.
    – Sumit Jain
    Aug 31, 2011 at 9:11
  • @KillianDS - In that example I'm passing the result as argument because I want to avoid the copies that'd occur if used as a return value.
    – brandx
    Aug 31, 2011 at 12:51
  • @brandx: return value optimization is likely to be applied, and at least give the C++11 alternative using rvalue references then. This is awful design that is the reason of so many unreadable APIs.
    – KillianDS
    Aug 31, 2011 at 13:45
  • @KillianDS - why rely on the compiler doing RVO? I'd much rather have the parameter as I provided here for the potential of reusing the vector elsewhere. As for C++11, it's cool and all, but C++11 was days old when the question was asked. If you have a C++11 alternative, please provide as an alternative answer.
    – brandx
    Dec 30, 2011 at 17:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.