dynamic char * help

[machikelxol]

I'm having a strange error when I try reading from a file. here is the
code:
buffstring values = 1, 2, 3, 4 and then it crashes afterwards on the
4th iteration.
This works fine until the fourth iteration.
breaks on this line: buffString = new char[numLineItems];
The contents of the text file it is reading:
1,1,1,1,1,1,1,1,1,5,6,7,8,9
2,2,2,2,2,2,2,2,2
3,3,3,3,3,3,3,3,3
4,4,4,4,4,4,4,4,4
5,5,5,5,5,5,5,5,5
6,6,6,6,6,6,6,6,6

Any ideas on how to fix this or what the problem may be? Need to get
the entire file read in and not error out on the fourth iteration.
(errors out on line with 5's on it)
Code is below:


void DrawWorld::LoadParseFile()
{

//string to store data in LoadParseFile(ifstream*) function
theFile = new ifstream("Maps\\1.txt");
char currentCharacter = '\0';
char *tokenBuffer = "";

int arraySize = 0;
int index = 0, innerindex = 0;

if (!theFile)
{
exit(1); // terminate with error
}

//deterimine size of file
while(!theFile->eof())
{
currentCharacter = theFile->get();

//increment our arraySize if we have a \n
if (currentCharacter == '\n')
{
arraySize++;
}
}

//there is typically one more
//row in a file than carriage returns
arraySize++;

//clear eof state
theFile->clear();
theFile->seekg(0);

/*initialize our arrays
the tiles*/
tiles = new int[arraySize];

//whether the tile is enterable
enterable = new int[arraySize];

/*whether the character is displayed
behind or in front of the tile*/
tileBehind = new int[arraySize];

//for storing animation on/off 1 = yes 0 = no
tileAnimationOnOff = new int[arraySize];

//whether the animation repeats or not 1 = yes 0 = no
tileAnimationLoop = new int[arraySize];

//for storing the time to wait in between animation switches
tileAnimationFrequency = new int[arraySize];

//for storing the animations
tileAnimations = new int*[arraySize];

while (!theFile->eof())
{
char *buffString;

//get a line out of mapFile and assign
//it to buffstring
char lineCounter[1];

int numLineItems = 0;
int currentLocation = theFile->tellg();

lineCounter[0] = theFile->get();

//determine number of characters in current line
while(lineCounter[0] != '\n')
{
lineCounter[0] = theFile->get();
numLineItems++;
}

int test = theFile->tellg();

//redimension our array to the right size for the line
buffString = new char[numLineItems];

//drop back the same number of characters
//that we just read
theFile->seekg(currentLocation);

//get the exact number of characters we need
theFile->getline(buffString, 100);

//first item is the tile number
tiles[index] = atoi(strtok(buffString, ","));

//next is enterable
enterable[index] = atoi(strtok(NULL, ","));

//tilebehind
tileBehind[index] = atoi(strtok(NULL, ","));

//tileAnimationOnOff
tileAnimationOnOff[index] = atoi(strtok(NULL, ","));

//tileRepeat
tileAnimationLoop[index] = atoi(strtok(NULL, ","));

//animation speed
tileAnimationFrequency[index] = atoi(strtok(NULL, ","));

//deterimine size of the inner array
int innerArraySize = 0, i = 0;

//grab the remaining characters in this line
tokenBuffer = strtok(NULL, "");

//begin the while with the first character of tokenBuffer
//TODO: only do this if tokenBuffer has a value
currentCharacter = tokenBuffer[0];

//while through the character array
while(currentCharacter != '\0')
{
//we are counting the commas, this will
//give us an accurate measurement of the
//elements we will have in our inner array
if (currentCharacter == ',')
{
innerArraySize++;
}

i++;

//select the next item in the character array
currentCharacter = tokenBuffer;
}

//we'll have 1 more element than we have commas
innerArraySize++;

//allocate memory for our inner array
tileAnimations[index] = new int[innerArraySize];

//assign the first item of the inner array
//TODO: make sure there is something in tokenBuffer first,
//then loop through it.
tileAnimations[index][innerindex] = atoi(strtok(tokenBuffer, ","));

//assign each item in the innerArray a value for
//the animation sequence.
for (innerindex++; innerindex < innerArraySize; innerindex++)
{
tileAnimations[index][innerindex] = atoi(strtok(NULL, ","));
}

/*buffString = new char[10];
delete [] buffString;*/

//increment index
index++;

}

}

 

[Stuart Redmann]

I'm having a strange error when I try reading from a file. here is the
code:
buffstring values = 1, 2, 3, 4 and then it crashes afterwards on the
4th iteration.
This works fine until the fourth iteration.
breaks on this line: buffString = new char[numLineItems];
The contents of the text file it is reading:
1,1,1,1,1,1,1,1,1,5,6,7,8,9
2,2,2,2,2,2,2,2,2
3,3,3,3,3,3,3,3,3
4,4,4,4,4,4,4,4,4
5,5,5,5,5,5,5,5,5
6,6,6,6,6,6,6,6,6

Any ideas on how to fix this or what the problem may be? Need to get
the entire file read in and not error out on the fourth iteration.
(errors out on line with 5's on it)
Code is below:


void DrawWorld::LoadParseFile()
{

[snipped _160_ lines of code]

}

Could you provide a _minimal_ (and above all compilable) example that
exposes the error?

Stuart

 

[Ian Collins]

I'm having a strange error when I try reading from a file. here is the
code:
buffstring values = 1, 2, 3, 4 and then it crashes afterwards on the
4th iteration.
This works fine until the fourth iteration.
breaks on this line: buffString = new char[numLineItems];
The contents of the text file it is reading:
1,1,1,1,1,1,1,1,1,5,6,7,8,9
2,2,2,2,2,2,2,2,2
3,3,3,3,3,3,3,3,3
4,4,4,4,4,4,4,4,4
5,5,5,5,5,5,5,5,5
6,6,6,6,6,6,6,6,6

Any ideas on how to fix this or what the problem may be? Need to get
the entire file read in and not error out on the fourth iteration.
(errors out on line with 5's on it)
Code is below:


void DrawWorld::LoadParseFile()
{

[snipped _160_ lines of code]

}

Could you provide a _minimal_ (and above all compilable) example that
exposes the error?

This is an excellent suggestion for two reasons:

1) It might produce an answer.
2) It will get the OP to think about breaking a way too long function
into sensible parts.

 

[James Bannon]

I'm having a strange error when I try reading from a file. here is the
code:

<SNIP>

Stuart is right, this is an awful lot of code to analyse to find out
what is wrong but I think a few remarks can be offered that might help.

void DrawWorld::LoadParseFile()
{

//string to store data in LoadParseFile(ifstream*) function
theFile = new ifstream("Maps\\1.txt");

Why are you using new here? AFAIK there is no need since the ifstream
constructor will handle this automatically anyway.

char currentCharacter = '\0';
char *tokenBuffer = "";

Overwriting a char const * (the type of the string literal "") will
result in undefined behaviour. Perhaps this is where the problem lies?

int arraySize = 0;
int index = 0, innerindex = 0;

I don't think these should be ints. Are you sure the buffers will not
overflow with this? arraySize definitely should not be an int.

if (!theFile)
{
exit(1); // terminate with error
}

This isn't necessary here since you're not using the non-throw version
of new above. If new fails you will get a bad_alloc exception, not a
null pointer return.

//deterimine size of file
while(!theFile->eof())
{
currentCharacter = theFile->get();

//increment our arraySize if we have a \n
if (currentCharacter == '\n')
{
arraySize++;
}
}

Well, OK I suppose but why don't you use vectors then you wouldn't need
to read the file in order to determine its size in lines? Also, all the
lines are created using new and that's an awful lot of memory to manage.
Are you sure you have all the proper calls to delete [] in this class?
Using vectors you would get the memory management for free.

while (!theFile->eof())
{
char *buffString;

Again, more explicit memory management when you don't need it. Use
std::string or vector< char > (assuming we're using the "C" locale
exclusively - it might be better to set the locale to be the native
locale here). This way you don't need to call delete later on in the
code as the string or vector will be destroyed automatically when it
goes out of scope.

//tileAnimationOnOff
tileAnimationOnOff[index] = atoi(strtok(NULL, ","));

atoi () is non-standard and doesn't do error checking very well.
Consider something like strtol, strtoul or some of the <locale>
conversion facilities in the standard library.

<SNIP>

In short, you've made a rod to break your own back with all the memory
management that is going on. Simplify by using the facilities available
to you in the standard library. That way there is less to go wrong.


Hope this helps,
Jim.

 

[Michiel.Salters]

I'm having a strange error when I try reading from a file. here is the
code:
buffstring values = 1, 2, 3, 4 and then it crashes afterwards on the
4th iteration.
This works fine until the fourth iteration.
breaks on this line: buffString = new char[numLineItems];

That is a very good indication what the problem is. You're using a
char[]
where you should be using a std::string (and std::vector and/or
std::list,
probably).

std::string takes care of memory management. Internally, it knows how
to
manage a char[]. Just don't try to access string when i >=
string.size(),
that's about all you need to know to use it safely.

See also the getline() variant that reads directly from stream to
std::string.

HTH,
Michiel Salters

 

[Marcus Kwok]

//tileAnimationOnOff
tileAnimationOnOff[index] = atoi(strtok(NULL, ","));

atoi () is non-standard and doesn't do error checking very well.
Consider something like strtol, strtoul or some of the <locale>
conversion facilities in the standard library.

While I agree with your other advice, atoi() is indeed standard (it is
in <cstdlib>). Another option is to use the stringstream technique
given in the FAQ.

 

[Noah Roberts]

<SNIP>

Stuart is right, this is an awful lot of code to analyse to find out
what is wrong.

I have adhd. When I see code like that my brain shuts off. I
literally can't read it. When I run into code like that in real code I
have to break it up.

 

[James Bannon]

//tileAnimationOnOff
tileAnimationOnOff[index] = atoi(strtok(NULL, ","));

atoi () is non-standard and doesn't do error checking very well.
Consider something like strtol, strtoul or some of the <locale>
conversion facilities in the standard library.

While I agree with your other advice, atoi() is indeed standard (it is
in <cstdlib>). Another option is to use the stringstream technique
given in the FAQ.

Ah... mea culpa. Can't be right all the time I suppose.

Jim.

 

[Old Wolf]

//deterimine size of file
while(!theFile->eof())

As well as what everyone else said: this is not the correct way
to read a file. eof() tells you if you have ALREADY tried to
read past the end of the file, and failed. It doesn't tell you if your
file pointer (if there is such a thing) has reached the end.

Instead, you should check each read operation for success or failure.

{
currentCharacter = theFile->get();

In this case:
while ( theFile->get(currentCharacter) )

istream::get(char) returns an indication of whether it succeeded.

//there is typically one more
//row in a file than carriage returns
arraySize++;

You should restructure your program to not use 'new'.

Also, instead of having a bunch of arrays each of this
dynamic length, you should create a structure with all
of the information for one record, and then have a
single array. If you use a standard container instead of
an array, then you do not have to count the file length
in advance. Look up std::vector or std::deque.