[Nuke]

ok, i have a class for 2d a vector image, anyway one of the constructors takes a filename and parses that file, resulting in a vector image object, in theory. anyway im scanning the file to figure out how much space needs to be malloced, but for some reason the counter isnt working. this is what the file looks like:

vertdefs
5 5
-5 -5
5 -5
-5 5
end

and this is part of the code to process it, minus the other half whick puts the values into the array and the rest of the class;

Code: [Select]


class vimage
{
public:
unsigned int numl;
sline* lines;

vimage( char file[32] )
{
numl = 0;
lines = NULL;

char data[8];
bool k = false;
printf("squeek %i \n", numl);

ifstream nvi(file);
nvi >> data;

if( strcmp(data, "vertdefs") == 0 )
{
while(!k && !nvi.eof())
{
nvi >> data;
printf("data: %s\n", data);

if(strcmp(data, "end") == 0 || nvi.eof())
{
k = true;
printf("meow %i \n", numl);
nvi.close();
}
else
{
numl++;
printf("woof %i \n", numl);
}
}
}
else
printf("vector image parsing error\n");

anyway this would go on to actually load the data in the file into a malloced array. only problem is the counter doesnt work. numl is the value in question. anyway ive set it to zero and when i loop through the file word by word, i know the loop runs 9 times. 8 for the 2 lines in the file (x,y coords for start and end points), and the 9th time the end token is read and the file is closed. now if the counter had done its job, it should read 8 (which would be divided by 4 to get the number of sline objects to be malloced for). anyway the print output looks something like this.

Code: [Select]

squeek 0
data: 5
woof -858993459
data: 5
woof -858993458
data: -5
woof -858993457
data: -5
woof -858993456
data: 5
woof -858993455
data: -5
woof -858993454
data: -5
woof -858993453
data: 5
woof -858993452
data: end
meow -858993452

will anyone please tell me what is going on here? cause i am totally confused, the compiler tosses zero warnings. the really funny thing is that i use the same meathod to parse 3d models and that code works.

[karajorma]

try it with printf %u instead of %i

You're using the token for a signed int with an unsigned int. The fact that you're getting negative print outs from an unsigned int is a definite clue as to what is wrong here even if that isn't the problem.

[Nuke]

tried that. also when i uncomment the malloc code that follows, it tries to allocate an insane amount of memory.

and it made the numbers bigger

Code: [Select]

squeek 0
data: 5
woof 3435973837
data: 5
woof 3435973838
data: -5
woof 3435973839
data: -5
woof 3435973840
data: 5
woof 3435973841
data: -5
woof 3435973842
data: -5
woof 3435973843
data: 5
woof 3435973844
data: end
meow 3435973844
Init time: 0.067000 seconds.
Starting master loop.
Average FPS: 58.003904


i actually figured it was a type problem and read up a considerable amount of info on the subject. but it seemed i did everything right. i tried various formatting tags on the printfs to try to figure out what i was doing wrong. this code works on the model parser as well, only thing different is the names of the variables. this is a really confusing glitch.

[phreak]

the strcmp is comparing the 8 character string "vertdefs" with the 8 character storage limit in the 'data' variable.  This leaves no room for the null terminator and therefore are overrunning.  Try using strncmp instead as it parses up to the programmer specified limit, or increase the size of the 'data' array.

And don't mix stdio and streamio, it makes satan emo.

[WMCoolmon]

Code: [Select]

class vimage
{
public:
unsigned int numl;

vimage( char file[32] )
{
numl = 0;

char data[16];
bool k = false;
std::cout << "squeek " << numl << std::endl;

std::ifstream nvi(file);
if(!nvi.is_open())
{
std::cout << "Couldn't open '" << file << "'" << std::endl;
return;
}
nvi.width(sizeof(data));
nvi >> data;

if( strcmp(data, "vertdefs") == 0 )
{
while(!k && !nvi.eof())
{
nvi.width(sizeof(data));
nvi >> data;
std::cout << "data: " << data << std::endl;

if(strcmp(data, "end") == 0 || nvi.eof())
{
k = true;
std::cout << "meow " << numl << std::endl;
nvi.close();
}
else
{
numl++;
std::cout << "woof " << numl << std::endl;
}
}
}
else
{
std::cout << "vector image parsing error" << std::endl;
}
}
};
For some reason the couts didn't work, so I may have made some dumb mistake in the rest of my program. (Or it may be something about the way the environment is set up, as I've been using this for Win32/SDL/OpenGL).

Anywho, I increased the buffer size and eliminated the possibility of an overrun with width() (see the str section of this page). I tossed in some sizeofs so that the buffer can be resized safely even if you forget about the rest of the code, and I added a check for if the file was open for obvious reasons. I also put brackets around the else statement. Superceding bracket style is making all paths of a loop use the same bracket style (or lack thereof) IMHO.

And I left this alone, but with the fixed-width font I'm using, it takes a split-second longer to tell the "1" from a lowercase "l", which has got to be the most ambiguous character in the Roman alphabet (given that it looks like an upper-case "I" in this font.) Not really something that's likely to cause problems since the compiler would catch it, but just something subtle I thought I'd call attention to.

EDIT: And I can vouch for phreak's statement on stdio and streamio. I forget what I was doing, but I have run into sync errors when I was using both. Generally, I just avoid streamio entirely. The syntax is pretty, but the ambiguity of what it's actually doing always bothers me...

[Nuke]

the strcmp is comparing the 8 character string "vertdefs" with the 8 character storage limit in the 'data' variable.  This leaves no room for the null terminator and therefore are overrunning.  Try using strncmp instead as it parses up to the programmer specified limit, or increase the size of the 'data' array.

i changed the size of the array and that seemed to fix it.

Code: [Select]

class vimage
{
public:
unsigned int numl;

vimage( char file[32] )
{
numl = 0;

char data[16];
bool k = false;
std::cout << "squeek " << numl << std::endl;

std::ifstream nvi(file);
if(!nvi.is_open())
{
std::cout << "Couldn't open '" << file << "'" << std::endl;
return;
}
nvi.width(sizeof(data));
nvi >> data;

if( strcmp(data, "vertdefs") == 0 )
{
while(!k && !nvi.eof())
{
nvi.width(sizeof(data));
nvi >> data;
std::cout << "data: " << data << std::endl;

if(strcmp(data, "end") == 0 || nvi.eof())
{
k = true;
std::cout << "meow " << numl << std::endl;
nvi.close();
}
else
{
numl++;
std::cout << "woof " << numl << std::endl;
}
}
}
else
{
std::cout << "vector image parsing error" << std::endl;
}
}
};
For some reason the couts didn't work, so I may have made some dumb mistake in the rest of my program. (Or it may be something about the way the environment is set up, as I've been using this for Win32/SDL/OpenGL).

Anywho, I increased the buffer size and eliminated the possibility of an overrun with width() (see the str section of this page). I tossed in some sizeofs so that the buffer can be resized safely even if you forget about the rest of the code, and I added a check for if the file was open for obvious reasons. I also put brackets around the else statement. Superceding bracket style is making all paths of a loop use the same bracket style (or lack thereof) IMHO.

And I left this alone, but with the fixed-width font I'm using, it takes a split-second longer to tell the "1" from a lowercase "l", which has got to be the most ambiguous character in the Roman alphabet (given that it looks like an upper-case "I" in this font.) Not really something that's likely to cause problems since the compiler would catch it, but just something subtle I thought I'd call attention to.

EDIT: And I can vouch for phreak's statement on stdio and streamio. I forget what I was doing, but I have run into sync errors when I was using both. Generally, I just avoid streamio entirely. The syntax is pretty, but the ambiguity of what it's actually doing always bothers me...

so in other words i need to pull out all stdio related stuff? like printfs and the like? thats probably a big undertaking, since i have of diagnostic messages as printfs. fortunately most of those are commented out. i currently have 3 file formats that need parsing, the vector font format, the model format, and the vector image format (this one). where the only one that works is the model format. this probibly explains why i never got the font parser to work. so i should probably fix that now before i continue working on other stuff. thanks guys.

[WMCoolmon]

If you wanted to cheat on the printf thing, you could make a function that takes a var_args as input and uses vsprintf to turn the printf stuff into a single string.

Code: [Select]

static char myBuf[1024];
void myPrintf(char *fmt, ...)
{
     va_list vl;
     va_start(vl, fmt);

     vsprintf(myBuf, fmt, vl);
     std::cout << myBuf;

     va_end(vl);
}
That way, at a future point you could throw in a toggle to store everything in a log file.

[Nuke]

id rather do it the right way. im doing it the wrong way because people who write tutorials cant agree on a way to do things. im currently trying to make my includes more c++ like. what includes do i need tor your above example to work?

[WMCoolmon]

<stdarg.h>

At a certain point there isn't really a right way, once you start abstracting things anyway. For a larger project it might be appropriate to have one big error/feedback function that has multiple levels of priority and precedence and redirects the output to multiple files depending on which subsystem it comes from, etc...

But what phreak and I are referring to is that mixing stdio and iostream has definite issues because they're two distinctive systems and so they may not line up properly. Just as an example, fwrite and fread don't necessarily access the hard drive every time they're called; they may or may not buffer stuff. If you really want to make sure that what's been done has been done when writing to disk, you have to call fflush(). So if you fwrite and then you try to read with a stream, you may find that what you just wrote doesn't appear. Whereas if you called fread, the internal handling knows that it needs to take into account that buffered data.

Similarly with cin and printf, I think there's some buffering going on. So if you cout and then you printf, your printf may end up showing up before the cout because of internal buffering or something weird like that. Or it may not show up at all.

So abstracting output isn't necessarily a bad thing, it just needs to be the same type of input/output.

[Nuke]

so how do i not mix stdio and iostream? thats the part im trying to figure out ive already started converting my printfs to std::couts, but im not sure what other functions are part of one and what are part of the other. (i think in the process i borked my model parser).

this is an sdl app so i really dont have any text output. for some reason an stdout.txt isnt being created (it used to be at some point but stopped working). then i started using a freopen ("****happens.txt","w",stdout); to create a ****happens.txt file, and it worked for awhile. that doesnt seem to work with the couts. any idea whats going on here?

[WMCoolmon]

fopen, printf, etc are all stdio. They're included from C headers, most notably stdio.h

iostream is included from C++ headers, like iostream, fstream etc.

Here's a page that I googled that lists C++ libraries. C libraries are listed at the bottom in their C++ form (<cstdio>, etc.)

Just pick one or the other and use it.

[Nuke]

side effect of all this is i completely revamped my model code, it actually makes since now. so im gonna rewrite that one using the same technique, which is the malloc some space, and load while youre counting, then realloc if you fill that space up. then realloc again at the end to kill any dirty space that may be left over when you find the end tag.

i replaced all the printfs with couts. and some of the strcmps with strncmps. i think i managed to purge the c related stuff, and moved ip to iostream instead of a crude combination of it and stdio. once my parsers are done i might look into messing around with some opengl.

[WMCoolmon]

malloc/free are C functions, new/delete are C++'s dynamic allocators... 

[Nuke]

damn it!

well since i'm only using a couple dynamic arrays i don't see now harm in moving over to the pure c++ ones.

what i've been doing is initializing the arrays with a fixed size before loading the data and realloc if that becomes full during the load. then when the model load is done i realloc again to crunch it down to size. is that way of doing things gonna work with c++'s replacement for malloc?

[phreak]

what i've been doing is initializing the arrays with a fixed size before loading the data and realloc if that becomes full during the load. then when the model load is done i realloc again to crunch it down to size. is that way of doing things gonna work with c++'s replacement for malloc?

you don't want to do that because the way you're using realloc(), it is going to delete memory allocated on the stack and that could/would give rise to errors, if you want it to work right, try this

Code: [Select]

const int ARRAY_DEFAULT = 16;
int array_size = ARRAY_DEFAULT;
float* list;

list = (float*)malloc(array_size * sizeof(float));

while (program_is_running)
{
    // ...
    // perform some operations here
    // ...

    if (memory_required > array_size)
    {
        //realloc if necessary
        while (memory_required > array_size)
        {
            array_size *= 2;
        }
        list = (float*)realloc(list, array_size);
    }

    // do more stuff...
}

free(list)


[Nuke]

my model loader essentially looks like this:

Code: [Select]



class model //this is our model class, it contains dynamic vert and linedef arrays.
{
public:
unsigned numl;
unsigned numv;
vec* verts;
unsigned* linea;
unsigned* lineb;

model( char file[32] ) //model objects that use this constructor are usually ones that are left in model space and are copied into other model objects to be transformed
{
numl = 0;
numv = 0;
verts = NULL;
verts_ms = NULL;
linea = NULL;
lineb = NULL;
unsigned msize = 256; //space for 256 objects will be allocated, if filled then it will allocate space for another 256 objects

cout << "Loading model: " << file << endl;

verts = (vec *)malloc(msize * sizeof(vec)); //now that we have our sizes of our defs, set length of dynamic arrays.
verts_ms = (vec *)malloc(msize * sizeof(vec));
linea = (unsigned *)malloc(msize * sizeof(unsigned)); //allocate initial block for line defs
lineb = (unsigned *)malloc(msize * sizeof(unsigned));

if (verts == NULL || verts_ms == NULL || linea == NULL || lineb == NULL)
{
cout << "Memory allocation error, aborting model load." << endl;
free(verts);
free(verts_ms);
free(linea);
free(lineb);
numl = numv = 0;
return;
}

char stage = 0;
char data[32];
ifstream nlm(file);

if(!nlm.is_open()) //idiot proofing
{
cout << "Couldnt open file: " << file << endl;
return;
}

while(!nlm.eof() && stage < 4) //loop through file word by word, allocate more memory as neccisary and load data
{
if(stage != 4)
nlm >> data;

if(stage==0) //find the vertdef start string
{
if(strncmp(data, "vertdefs", 8)==0)
stage++;
}
else if(stage==1) //count data
{
if(strncmp(data, "end", 3)==0)
{
stage++; //once we see the end we stop counting and up our stage
msize = 256; //reset this so we can allocate line space as needed
}
else
{
if (numv == msize)
{
msize += 256;
verts = (vec *)realloc(verts, msize * sizeof(vec));
verts_ms = (vec *)realloc(verts_ms, msize * sizeof(vec));
cout << "Too many verts, reallocating memory." << endl;

if (verts == NULL || verts_ms == NULL)
{
cout << "Memory allocation error, aborting model load." << endl;
free(verts);
free(verts_ms);
free(linea);
free(lineb);
numl = numv = 0;
return;
}
}

istringstream fdat(data);
fdat >> verts[numv].x;
nlm >> verts[numv].y >> verts[numv].z;
verts_ms[numv] = verts[numv];
cout << "Vert " << numv << " set to " << verts[numv].x << ", " << verts[numv].y << ", " << verts[numv].z << endl;
numv++;
}
}
else if(stage==2) //find linedef start string
{
if(strncmp(data, "linedefs", 8)==0)
stage++;
}
else if(stage==3) //count more data
{
if(strncmp(data, "end", 3)==0)
stage++; //once we see the end we stop counting and up our stage
else
{
if (numl == msize)
{
msize += 256;
linea = (unsigned *)realloc(linea, msize * sizeof(unsigned));
lineb = (unsigned *)realloc(lineb, msize * sizeof(unsigned));
cout << "Too many line defs, allocating more memory." << endl;

if (linea == NULL || lineb == NULL)
{
cout << "Memory allocation error, aborting model load." << endl;
free(verts);
free(verts_ms);
free(linea);
free(lineb);
numl = numv = 0;
return;
}
}


istringstream idat(data);
idat >> linea[numl];
nlm >> lineb[numl];
cout << "Vert indices for line " << numl << " set to: " << linea[numl] << ", " << lineb[numl] << endl;
numl++;
}
}
}

verts = (vec *)realloc(verts, numv * sizeof(vec)); // compact the size of the memory blocks
verts_ms = (vec *)realloc(verts_ms, numv * sizeof(vec));
linea = (unsigned *)realloc(linea, numl * sizeof(unsigned));
lineb = (unsigned *)realloc(lineb, numl * sizeof(unsigned));

if (verts == NULL || verts_ms == NULL || linea == NULL || lineb == NULL)
{
cout << "Memory allocation error, aborting model load." << endl;
free(verts);
free(verts_ms);
free(linea);
free(lineb);
numl = numv = 0;
return;
}

nlm.close();
cout << "Model load complete." << endl;
}

//the rest of the class goes here

so its sorta the same thing right?

[WMCoolmon]

The reason that you want to use new[] and delete[] with C++ is that new and delete call the constructors for the classes that they're used with. Malloc/realloc do not.

You can get around that by setting values manually, but it's not really what you're supposed to do with a C++ program.

[Nuke]

its my fist game engine

[Colonol Dekker]

This whole thread makes my head hurt............

All programs should be made in FRED2. (without variables)

[Nuke]

frankly id rather deal with c++ than sexps, ive hated those things from day one. **** starcraft had a better events system.

anyway i red up on new and delete and i like the way it lets me just set an exception so i dont have to idiot check every malloc/realloc call. then i can cout to tell the programmer/user hes a moron and kill the program