Wow. So many magic numbers. I would suggest you immediately
convert any hard-coded number you see in that code to refer
to static const int, static const unsigned int or #define
at the top of your source file.
You're allocating a massive amount of memory here, but your
problem is on free and you're checking all your allocation
commands, so that shouldn't be a problem.
Your key-scanning routine will be much more user-friendly if you
use strtok. You can pass '~' as a token, and won't need to have
the use specify in two places (i.e. the string of keys and the
key count) how many keys there are. I got some nifty access
violations till I figured out I needed to end the list with a
tilde as well.
You're opening and closing the file each time through LoadList.
Is this a good idea? Of course, you're reading 6 megs at a
time, but in a 600meg file, thats 100 opens & closes for no
reason.
You're using the old ifstream, i.e. the one in "fstream.h". I
can tell because nocreate isn't a standard flag. I'd suggest
using "fstream" and the std::ifstream, since it's actually
standard. It's also a bit weird that you're mixing C++
libraries and C libraries, but that's OK I guess.
You're not checking for fail after TheList.read, nor are you
making sure you're not trying to read past the end of the file.
For the file I used (1 meg), it did indeed fail because reading
more than what's left is a failure. If you're using the
standard fstream, this will cause subsequent calls to fstream to
return odd values, i.e. TheList.tellg () returns -1. This
doesn't seem to happen for the old nonstandard fstream you're
using, though.
In LoadFile, again while reading keys into FileData, strtok
would help a lot. Also, you seem to be inserting an off-by-one
error when you return the value from LoadFile (returns 9999).
But you don't use the return value, so no point.
Back in StrainList, what is the purpose of these lines?:
strcpy(FileBuffer,FileFrame[i]);k+=(long)strstr(FileFrame[i],KeyFrame[j]);strcpy(FileBuffer,FileFrame[i]);
|
The first line copies the current FileFrame into your
FileBuffer. The second line increments k if FileFrame
is<br>in KeyFrame[j], but doesn't touch FileBuffer. The third line<br>recopies it. In fact, you copy data into FileBuffer all over<br>the place, but it's never used except by the LoadFile function.<br><br>This function is spaghetti code of the worst kind. It doesn't<br>surprise me that it breaks. I wasn't able to get it to break<br>for my one meg file with a couple of keys, so there must be some <br>combination of the keys you use and the data you have that <br>breaks your code.<br><br>The problem is not with free–it's with your slop. Since the <br>mallocs and frees all look right, there's probably a logical <br>combination of your plethora of inputs that causes some array to <br>overflow its bounds, causing pointers to be overwritten that you <br>later try to delete. To check this, output the values of your <br>pointers when you malloc, then output them as you try to free. <br>I'm willing to bet they don't match.<br><br>Edited by - Stoffel on June 5, 2001 4:45:27 PM<br><br>Edited by - Stoffel on June 5, 2001 4:50:04 PM