C++ don't know why content is changed

Question

The code is from my friend. Please have a look at it and give us some suggestions.

in VoltageForDisplayAndAnalyze.h file

private:

string *    grpVolFileName[ MAX_NUM_OF_GROUP ]; //for voltage files
fstream *   aStream_grpVolt[ MAX_NUM_OF_GROUP ];

in VoltageForDisplayAndAnalyze.CPP file

void VoltageForDisplayAndAnalyze::MakeFileNameForGroupCell( int xgrpIndex )
{
   int numOfFiles = (EEnd - EBegin + 1) / 255;
   grpVolFileName[ xgrpIndex ] = new string[ numOfFiles ];      
   assert( grpVolFileName[ xgrpIndex ] != NULL );
   aStream_grpVolt[xgrpIndex] = new fstream[ numOfFiles ];  
   assert( aStream_grpVolt[ xgrpIndex ] != NULL );
   .....
   for( int fileIndex=0; fileIndex<numOfFiles; fileIndex++ )
   {            
    char * tempbuf = new char[256];
    memcpy(tempbuf, mPath.c_str(), mPath.length() );
    tempbuf[mPath.length()] = '\0';
    char * numChar = new char[4];
    itoa(xgrpIndex, numChar, 3 );
    numChar[3] = '\0';   

    strcat(tempbuf, numChar );
    grpVolFileName[ xgrpIndex ][ fileIndex ].assign(tempbuf );

    }    
     ....
}

we print out in above method which assign values to grpVolFileName, the result looks good. These values are location of some files. Then the following method is called to use above grpVolFileName array of string pointers. It is good for first 4 and then the print out show the directory cannot being print out correctly, some squares are print out and error happened. But it is not always at the 4th, sometime it happens at very begining.

void VoltageForDisplayAndAnalyze::SaveDataForGrpCell( int grpIndex, int fileIndex, int xBegin, int xEnd     {

    int i = 0;
    char answer;

//save to hard disk
aStream_grpVolt[grpIndex][fileIndex].open(  grpVolFileName[grpIndex][fileIndex].c_str() , fstream::out | std::ofstream::app );
cout << "open file: grpIndex= " << grpIndex <<"  fileIndex= " << fileIndex << " :  " << grpVolFileName[grpIndex][fileIndex] << " to save." << endl;
if( aStream_grpVolt[grpIndex][fileIndex].good() )
{   
   //.....  
}
else
{

    cout << "Cannot open file: " << grpVolFileName[grpIndex][fileIndex] << endl;
   }

   //.......
}

print out from above method like those:

...
open file: grpIndex= 2  fileIndex= 0 :  C:\Workspace\UncusJava\DataFiles\spc1\2 to save.
open file: grpIndex= 3  fileIndex= 0 :  C:\Workspace\UncusJava\DataFiles[][][][]4@ to save.

that means the grpVolFileName array has been changed in the process. It causes this if clause, if( aStream_grpVolt[grpIndex][fileIndex].good() ) , returns flase and error happen.

I searched entire solution (Visual Studio 2008), above two methods are only places use the grpVolFileName array of pointers. It is a single thread for this part.

Any comments?

Answer 1

I read twice your question and still I'm not sure what you're trying to achieve. From what I understand you are essentially traversing a directory hierarchy. It (nearly) doesn't matter what platform you're working on: there are probably better tools for the job than C++. Chances are you can bang a script faster, more easily, that is shorter in code and that is correct for performing the same task.

Answer 2

I'm sorry to say that your friend's code is a real mess. I suspect you are corrupting your strings in the for loop. You could try to rewrite it as:

In VoltageForDisplayAndAnalyze.cpp:

void VoltageForDisplayAndAnalyze::MakeFileNameForGroupCell(int xgrpIndex)
{
    int numOfFiles = (EEnd - EBegin + 1) / 255;
    grpVolFileName[xgrpIndex] = new string[numOfFiles];
    aStream_grpVolt[xgrpIndex] = new fstream[numOfFiles];
    // The asserts are not needed: new will throw an exception
    // if the allocation fails.
    ...
    for (int fileIndex = 0; fileIndex < numOfFiles; fileIndex++)
    {            
        // You should use std::string instead of C strings whenever
        // possible (and it means almost always: std::string can
        // interoperate with C strings).
        string tempbuf = mpath;

        // This is the C++ way of converting numbers to strings.
        ostringstream oss;
        oss << xgrpIndex;
        string numChar = oss.str();

        tempbuf += numChar;
        grpVolFileName[xgrpIndex][fileIndex] = tempbuf;

    }
    ...
}

// Remember to delete the arrays you have allocated with new:
// delete[] grpVolFileName[xgrpIndex];
// delete[] aStream_grpVolt[xgrpIndex];

While still not clean and concise (the code in the for loop is quite redundant, and we could have used std::vector instead of C arrays), this code now is guaranteed to assign correct data to grpVolFileName[xgrpIndex][fileIndex].

Try to run this code and see if your error has been solved.

Answer 3

Although the code is ugly and apparently has memory leaks, I don't see anything that would be corrupting your strings. It's possible that they're getting overwritten elsewhere in code.

You could test this by setting up a data breakpoint in Visual Studio to break whenever the string is modified.

Software like Valgrind (not available for Windows) and Purify (commercial) can also help detect memory corruption like this.