malloc() returning null for no reason 1

[zanza]

ftp://tricks:tricks@67.18.198.26/patrick/engine.zip

the malloc on line 86 of nav.c returns null while, not only is its parameter valid, but, i also use the exact same implementation nearer the top of the file.

---

for any of you who might keep track... yeah, i revived my oldest project again. i _still_ dont entirely know what im doing.

žÅNžÅ

 

[Salem]

First I looked at your code, and decided that it would probably crash as you described because there is NO error checking at all inside the code.

Well I ran your code, and it worked perfectly.
It even worked perfectly when I ran it with electric fence to trap mis-use of allocated memory.

Then I ran it with test coverage, and now I see what the problem is.

   5    fscanf(level, "%d", &room[i].numberofDialogs);

  35    for(j = 0; j < 6; j++)
        {
  30        fscanf(level, "%d", &room[i].dir[j].numberOfDoors);
  30        if(room[i].dir[j].numberOfDoors)
            {
####            room[i].dir[j].door = malloc(sizeof(room[i].dir[j].door) * room[i].dir[j].numberOfDoors);

The #### means that line of code was never executed.

To me, this means that the fscanf for the numberOfDoors failed, and in doing so, left numberOfDoors uninitialised.

Now on my system, it looks like malloc is returning memory filled with zeros, because the if statement fails. So all the interesting inner loops which allocate more memory never get called here.

On your system, it looks like your malloc returns memory filled with junk, and anything non-zero will make your if statement succeed. This includes stupidly large and/or negative values which will definitely cause malloc to break.

In short, your code is extremely sensitive to the format of your file. A one character mistake anywhere and the whole lot comes crashing down. You definitely need a more robust way of reading files.

Something like this
> fscanf(level, "%d", &numberOfRooms);
Would be

char buff[BUFSIZ];
if ( fgets( buff, BUFSIZ, level ) != NULL ) {
  if ( sscanf( buff, "%d", &numberOfRooms ) == 1 ) {
    if ( numberOfRooms > 0 && numberOfRooms < MAX_ROOMS ) {
    }
  }
}

Even then, sscanf() can't detect integer overflow, so the most robust solution would use strtol().

--

 

[zanza]

well, im glad it worked despite my lack of error checking.

but, thats not the troublesome part of the code. first though, i updated one small error i found in the version i first posted... the link above contains the updated source.

if(room[i].numberofDialogs)
{
	room[i].dialog = malloc(sizeof(room[i].dialog) * room[i].numberofDialogs);

	for(j = 0; j < room[i].numberofDialogs; j++)
	{
		fscanf(level, "%d", &room[i].dialog[j].numberOfSections);
		fscanf(level, "%d", &room[i].dialog[j].start);

		fscanf(level, "%d", &strLength);
		room[i].dialog[j].name = malloc(sizeof(char) * strLength);
		fgetc(level);
		fgets(room[i].dialog[j].name, strLength, level);

this is the area that breaks. the very last malloc in this list (line 86 in nav.c) returns null, even when passed straight ints.

oh, and on my machine, every variable (including numberOfDoors) has a value. :\

as to the stability/robustness of my reading in method... yeah. ^^; im just going for simple now. i have the dedication/utilities to catch most reading errors (at least i have thus far), so ill just leave it this way until i can get it to work.

a programmer friend of mine suggested (without having looked at the source) that i may be running out of memory. i really dont think that is the case... as i have 640 megs of ram and the application is only taking up just over a meg of memory when it reaches that point in the code.

 

[Salem]

room[i].dir[j].door = malloc(sizeof(room[i].dir[j].door) *
    room[i].dir[j].numberOfDoors);
room[i].dialog = malloc(sizeof(room[i].dialog) *
    room[i].numberofDialogs);
room[i].dialog[j].section = malloc(sizeof(room[i].dialog[j].section) *
    room[i].dialog[j].numberOfSections);
room[i].dialog[j].section[k].answer = malloc(sizeof(room[i].dialog[j].section[k].answer) *
    room[i].dialog[j].section[k].numberOfAnswers);

These 4 malloc calls are allocating the wrong amount of memory - in all cases, too little memory.
The result being the memory pool is being trashed, so at some point it all blows up despite all the parameters appearing to be correct.

You need the size of what the pointer points to, not the size of the pointer (as you have it in these cases).
In general, the call is this

p = malloc( howmany * sizeof *p );

Not this

p = malloc( howmany * sizeof p );

--

 

[zanza]

i was wondering about that earlier last night when i started writing this... hehe.

thank you salem.