Memory Heap Problem

Started by
12 comments, last by RichardForgacs 12 years, 7 months ago
Hi!

I have a strange problem with a part of my code. I wrote an .ASE file loader to my application with the help of a book, but when I try to start my code without Debuging, it says that: "Unhandled exception at 0x77a737b7 in gl5.exe: 0xC0000374: A heap has been corrupted." I started to debug my code and i found stranges errors :S Most of the error messages are: "CXX0030: Error: expression cannot be evaluated". It's about pointers which i was deleted before. I tried everything, but can't find the problem. I need your help! Here's my code:

ase.cpp:


#include <iostream>
#include <stdio.h>
#include "ase.h"

using namespace std;

MODEL::MODEL(){mdl_objects = NULL;max_objects = 0;}
MODEL::~MODEL(){}

int countStrings (char *buffer, char *szo);

bool MODEL::Load(char * name){
FILE * file = NULL;
long filesize;
char *buffer;
char *tmp,token[] = " \\\":\t\r\n";
long cur_object = -1;

Release();

file = fopen(name,"rb");
fseek(file,0,SEEK_END);
filesize = ftell(file); //fájl mérete

buffer = new char[filesize+5];
strcat(buffer," EOF");
fseek(file,0,SEEK_SET);
fread(buffer,1,filesize,file);
fclose(file);

max_objects = countStrings(buffer,"*GEOMOBJECT");
mdl_objects = new MODEL_OBJECT[max_objects+1];

tmp = strtok(strdup(buffer),token);

while(strcmp(tmp,"EOF")){
if(strcmp(tmp,"*GEOMOBJECT") == 0){
cur_object++;

mdl_objects[cur_object].max_vertices = 0;
mdl_objects[cur_object].max_triangles = 0;
//mdl_objects[cur_object].max_uv_coord = 0;
mdl_objects[cur_object].vertex = NULL;
mdl_objects[cur_object].triangle = NULL;
//mdl_objects[cur_object].uv_coord = NULL;
mdl_objects[cur_object].max_facenormal = 0;
mdl_objects[cur_object].face_normal = NULL;
}
else if(strcmp(tmp,"*MESH_NUMVERTEX") == 0)
{
mdl_objects[cur_object].max_vertices = strtol(strtok(NULL,token),NULL,10);
mdl_objects[cur_object].vertex = new MODEL_VERTEX[mdl_objects[cur_object].max_vertices+1];
}
else if(strcmp(tmp,"*MESH_NUMFACES") == 0){
mdl_objects[cur_object].max_triangles = strtol(strtok(NULL,token),NULL,10);
mdl_objects[cur_object].max_facenormal = mdl_objects[cur_object].max_triangles;
mdl_objects[cur_object].triangle = new MODEL_TRIANGLE[mdl_objects[cur_object].max_triangles+1];
mdl_objects[cur_object].face_normal = new MODEL_FACE_NORMAL[mdl_objects[cur_object].max_facenormal+1];
}
else if(strcmp(tmp,"*MESH_VERTEX") == 0){
long cur_vertex = strtol(strtok(NULL,token),NULL,10);

sscanf(strtok(NULL,token),"%f",&mdl_objects[cur_object].vertex[cur_vertex].xyz[0]);
sscanf(strtok(NULL,token),"%f",&mdl_objects[cur_object].vertex[cur_vertex].xyz[2]);
sscanf(strtok(NULL,token),"%f",&mdl_objects[cur_object].vertex[cur_vertex].xyz[1]);
}
else if(strcmp(tmp,"*MESH_FACE") == 0){
long cur_tri = strtol(strtok(NULL,token),NULL,10);

strtok(NULL,token);
mdl_objects[cur_object].triangle[cur_tri].point[0] = strtol(strtok(NULL,token),NULL,10);
strtok(NULL,token);
mdl_objects[cur_object].triangle[cur_tri].point[1] = strtol(strtok(NULL,token),NULL,10);
strtok(NULL,token);
mdl_objects[cur_object].triangle[cur_tri].point[2] = strtol(strtok(NULL,token),NULL,10);
}
/*else if(strcmp(tmp,"*MESH_NUMTVERTEX") == 0){
mdl_objects[cur_object].max_uv_coord = strtol(strtok(NULL,token),NULL,10);
mdl_objects[cur_object].uv_coord = new MODEL_UV_COORD[mdl_objects[cur_object].max_uv_coord+1];
}
else if(strcmp(tmp,"*MESH_TVERT") == 0){
long cur_tver = strtol(strtok(NULL,token),NULL,10);

sscanf(strtok(NULL,token),"%f",&mdl_objects[cur_object].uv_coord[cur_tver].uv[0]);
sscanf(strtok(NULL,token),"%f",&mdl_objects[cur_object].uv_coord[cur_tver].uv[1]);
}*/

else if(strcmp(tmp,"*MESH_FACENORMAL") == 0){
long cur_fnormal = strtol(strtok(NULL,token),NULL,10);

sscanf(strtok(NULL,token),"%f",&mdl_objects[cur_object].face_normal[cur_fnormal].xyz[0]);
sscanf(strtok(NULL,token),"%f",&mdl_objects[cur_object].face_normal[cur_fnormal].xyz[1]);
sscanf(strtok(NULL,token),"%f",&mdl_objects[cur_object].face_normal[cur_fnormal].xyz[2]);
}

tmp = strtok(NULL,token);
}

delete [] buffer;
buffer = NULL;

return true;
}

int countStrings (char *buffer, char *szo){
char token[] = " \\\":\t\r\n";
char *ptr = strtok(strdup(buffer),token);

int i = 0;

while (strcmp(ptr,"EOF")){
if(strcmp(ptr,szo) == 0) i++;
ptr = strtok(NULL,token);
}
return i;
}

void MODEL::Release(){
if( max_objects > 0){
for(long obj = 0; obj < max_objects; obj++){
if(mdl_objects[obj].max_vertices > 0) delete [] mdl_objects[obj].vertex;

if(mdl_objects[obj].max_vertices > 0) delete [] mdl_objects[obj].triangle;

if(mdl_objects[obj].max_facenormal > 0) delete [] mdl_objects[obj].face_normal;
// if(mdl_objects[obj].max_vertices > 0) delete [] mdl_objects[obj].uv_coord;
}
delete [] mdl_objects;
mdl_objects = NULL;
max_objects = 0;
}

}



ase.h
/*struct MODEL_MATERIAL{
char *file_name;
long tex_id;
};*/

struct MODEL_VERTEX{
float xyz[3];
};

/*struct MODEL_UV_COORD{
float uv[5];
};*/

struct MODEL_TRIANGLE{
long point[3];
long uv_point[3];
};

struct MODEL_FACE_NORMAL{
float xyz[3];
};

struct MODEL_OBJECT{
long max_triangles;
long max_facenormal;
long max_vertices;
//long max_uv_coord;
//long material;
MODEL_FACE_NORMAL *face_normal;
MODEL_VERTEX *vertex;
MODEL_TRIANGLE *triangle;
//MODEL_UV_COORD *uv_coord;
};

class MODEL{
public:
MODEL();
~MODEL();
MODEL_OBJECT *mdl_objects;
long max_objects;
bool Load(char *file_name);
void Release();
};



Advertisement
[source]
buffer = new char[filesize+5];
strcat(buffer," EOF");
fseek(file,0,SEEK_SET);
fread(buffer,1,filesize,file);
fclose(file);
[/source]


Hi,
I'm not sure this is the problem but are you sure this does what it is supposed to?

As far as I can see you are appending the string " EOF" to an uninitialized memory area.
You could try something like this

strcpy(buffer + filesize, " EOF");

instead of

strcat(buffer, " EOF");

Maybe that helps rolleyes.gif

[source]
buffer = new char[filesize+5];
strcat(buffer," EOF");
fseek(file,0,SEEK_SET);
fread(buffer,1,filesize,file);
fclose(file);
[/source]


Hi,
I'm not sure this is the problem but are you sure this does what it is supposed to?

As far as I can see you are appending the string " EOF" to an uninitialized memory area.
You could try something like this

strcpy(buffer + filesize, " EOF");

instead of

strcat(buffer, " EOF");

Maybe that helps rolleyes.gif


I think that could be it. if you use strcat (meaning you're concatenating something a string terminated with a 0) on an uninitalized string, the buffer could be filled with all 0xFF's, and strcat will just run along in memory until it comes ot a 0, which may be far past the size of the buffer.

My Gamedev Journal: 2D Game Making, the Easy Way

---(Old Blog, still has good info): 2dGameMaking
-----
"No one ever posts on that message board; it's too crowded." - Yoga Berra (sorta)

Thanks a lot, that was also a mistake from me in the code. The role of this piece of [color="#1c2837"]code ([font=Consolas,]strcat(buffer," EOF"); ) is to mark the end of the file and i can found it easily in the loop. But the program still craches when I try to run it without debugging. :( [/font]
The new code looks liek this:

buffer = new char[filesize+5];
fseek(file,0,SEEK_SET);
fread(buffer,1,filesize,file);
fclose(file);

strcat(buffer," EOF");



I think it's correct now, but maybe I'm wrong. Any other ideas?
[font="verdana, arial, helvetica, sans-serif"]
Return Value of fread();
The total number of elements successfully read is returned as a size_t object, which is an integral data type.
If this number differs from the count parameter, either an error occured or the End Of File was reached.
You can use either ferror or feof [/font][font="verdana, arial, helvetica, sans-serif"]to check whether an error happened or the End-of-File was reached. [/font]

[/quote]

So.. basically..



int bytes = 1;
int bytesRead = 0;

file = fopen(path);

if (file != NULL)
{
if (fseek(file, 0, SEEK_END) == 0)
{
int filesize = ftell(file);
char* buffer = new char[filesize + 5];


if (fseek(file, 0, SEEK_SET) == 0) //I guess this isn't all that necessary to check if you could move "forward"
{

while (!feof(file)) //You could just do a counter for the filesize but feof handles everything for ya just fine.
{
bytesRead = fread(buffer, bytes, filesize, file);
}

if (fclose(file) != 0);
{
//Do some error handling if you cannot close the file..
}
}
}
}



In short: Don't foget error checking! Good books should be reminding you of this constantly.


In short: Don't foget error checking! Good books should be reminding you of this constantly.


This is the catch 22 of trying to write code samples in book form. Space is always limited and you want the sample to be as concise and on topic as possible. The first victim, comments ( which makes sense as the book you are reading should in fact be the comments ), the second... error checking. This is even more true in Java books where error checking can be a bit more... verbose.


In many ways, its a problem with no easy solution. Well except I suppose to use websites instead of books. :)

[quote name='Fekete' timestamp='1314199060' post='4853236']
In short: Don't foget error checking! Good books should be reminding you of this constantly.


This is the catch 22 of trying to write code samples in book form. Space is always limited and you want the sample to be as concise and on topic as possible. The first victim, comments ( which makes sense as the book you are reading should in fact be the comments ), the second... error checking. This is even more true in Java books where error checking can be a bit more... verbose.


In many ways, its a problem with no easy solution. Well except I suppose to use websites instead of books. :)
[/quote]

I agree with you whole heartedly. This day and age, I think books are (becoming) obsolete; especially when it comes to programming information. But I digress; I haven't read many programming related books to know the general pattern of writing, so I'll get off my high pony. :)

For the OP; I suggest you research and implement a simple logging class for yourself, send me a PM if you'd like some help. Also research your functions/classes and understand their return values. Then if something fails, write it out in english (or your preferred language) to a file you can check. (i.e. "[date-time]: (function-name) (function could not read file [filename])"). This will save you many hours of headache. :-)
Thanks for the answers, but the program still crashers. :(

The new code looks liek this:

buffer = new char[filesize+5];
fseek(file,0,SEEK_SET);
fread(buffer,1,filesize,file);
fclose(file);

strcat(buffer," EOF");



I think it's correct now, but maybe I'm wrong. Any other ideas?



You still suffer from uninitialized data at the end of buffer. As BeerNuts said the strcat function will work its way through the buffer looking for the first zero byte it finds, and start appending at that point. In other words, it assumes that all strings are null terminated.
The problem here is that the fread function does not append a zero byte at the end of the buffer the way strcpy and strcat does.
This means that when the strcat function is looking for that trailing zero it may continue past the end of the buffer looking for it, and once it happens to find a zero somewhere past the end of buffer, and writes to that memory, the OS will interrupt your program and spit out a error message typically containing words like Access, Violation, Memory and the like.

You can fix this either by filling buffer with zeros before using it.

memset((void*)buffer, 0, sizeof(buffer));

or you can add a zero at the end manually after reading the file with fread.

In the latter case the return value from fread comes in handy, as Fekete mentioned.

buffer[bytesRead] = '\0';
strcat(buffer, " EOF");
...

This topic is closed to new replies.

Advertisement