Allocate space for 2d array

I'm having problems allocating space for a 2d array. Since I don't know precisely where the problem is, I've posted the exact content of the function I'm working with as well as the file that it references. Basically what happens is that I'm always ending up with junk values in my array. I figure it's either a memory size problem or something to do with the way that my typedef is set up. Any help would be appreciated.

Code:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

/* Each of the rooms is a row of the array
and each room attribute is a column of the array */
/* Room attributes:
 * column 0	Room Number
 * column 1	Location of room in room file (character offset)
 * column 2	Number of the room to the north
 * column 3	Number of the room to the east
 * column 4	"	"	"	" south
 * column 5	"	"	"	" west
 * column 6	"	"	"	" up
 * column 7	"	"	"	" down
 */

#define ROOM_ATTRIBUTES 8

/* The pointer to the room array needs to be passed to other functions,
 * so this typedef is made in the header file
 */
typedef int (*rmArray)[ROOM_ATTRIBUTES];

rmArray Parse_Room_List(FILE *roomFile) {
        int nextChar;
        char nextLine[100];
        int roomIndex = 0;

	/* This is supposed to initialize the size of the room array.
	 * I expect my problem may be here.
	 */
        rmArray roomInfo = malloc(sizeof(int)*ROOM_ATTRIBUTES);
        for(;;) {
                nextChar = fgetc(roomFile);

		/* If the next character is a "#" character, then the number that follows
		 * is a room number
		 */
                if (nextChar == '#') {

			/* This resizes the array based on the number of rooms logged.
			 * I expect I may have a problem here too.
			 */
                        roomInfo = realloc(roomInfo,(roomIndex+1)*sizeof(int)*ROOM_ATTRIBUTES);

			/* Gets the rest of the line, extracts the number, and stores it in
			 * the 0th attribute column for the current room index.
			 */
                        fgets(nextLine,sizeof(nextLine),roomFile);
                        sscanf(nextLine,"%d",&roomInfo[roomIndex][0]);

			/* Gets the current position in the file and stores it in the 1st
			 * attribute column.
			 */
                        roomInfo[roomIndex][1]=(int) ftell(roomFile);
                        roomIndex++;
                }

		/* If the next character is a "@" character, then the number that follows
		 * is an attribue for the most recent room
		 */
                else if (nextChar == '@') {
                        roomIndex--;

			/* nextChar stores the room attribute letter (n, e, s, w, u, or d)
			 * and nextLine stores the rest of the line following that character
			 */
                        nextChar = fgetc(roomFile);
                        fgets(nextLine,sizeof(nextLine),roomFile);

			/* Depending on which character was read, the number in the line
			 * is extracted and stored in the appropriate attribute column.
			 */
                        switch(nextChar){
                                case 'n':
                                        sscanf(nextLine,"%d",&roomInfo[roomIndex][2]);
                                        break;
                                case 'e':
                                        sscanf(nextLine,"%d",&roomInfo[roomIndex][3]);
                                        break;
                                case 's':
                                        sscanf(nextLine,"%d",&roomInfo[roomIndex][4]);
                                        break;
                                case 'w':
                                        sscanf(nextLine,"%d",&roomInfo[roomIndex][5]);
                                        break;
                                case 'u':
                                        sscanf(nextLine,"%d",&roomInfo[roomIndex][6]);
                                        break;
                                case 'd':
                                        sscanf(nextLine,"%d",&roomInfo[roomIndex][7]);
                                        break;
                        }
                        roomIndex++;
                }
                else if (nextChar == EOF)
                        break;
        }
        return roomInfo;
}

Here's the kind of file that this function will be reading. The room descriptions are not used by this function, they're just there for completeness.

Code:

#1
@n5
This is the room description for room 1.
Room 5 is to the north of here.

#2
@e5
This is the room description for room 2.
Room 5 is to the east of here.

#3
@s5
This is the room description for room 3.
Room 5 is to the south of here.

#4
@w5
This is the room description for room 4.
Room 5 is to the west of here.

#5
@n3
@e4
@s1
@w2
This is the room description for room 5.
Rooms 1 through 4 are in each of the cardinal directions.

Code:

int **twodee;
...
twodee = malloc( ROWS * sizeof *twodee );
...
for( x=0; x<ROWS; x++ )
{
    twodee[ x ] = malloc( COLS * sizeof **twodee );
}

Basically, you want that. Or, you want something like this:

Code:

while( fgets( fp, buf, BUFSIZ ) != NULL )
{
    rowcount++;
    temp = realloc( twodee, rowcount * sizeof *twodee );
    if( temp )
    {
        twodee = temp;
        twodee[ rowcount ] = malloc( COLS * sizeof **twodee );
        if( twodee[ rowcount ] )
            ...sscanf your buffer into each column of this row...
    }
}

Not exactly, because you're not just reading a row of numbers, but something to that effect.


Quzah.

> rmArray roomInfo = malloc(sizeof(int)*ROOM_ATTRIBUTES);
A couple of points
1. To ensure you have the right amount of space, you should do this:
rmArray roomInfo = malloc( sizeof(*roomInfo ) );

2. The space returned by malloc is NOT initialised. I notice in your nsewud detection that you only end up reading values for valid rooms. The rest of the rooms will contain junk (not zeros say).

> for(;;)
Read the file like this.

Code:

while ( fgets(nextLine,sizeof(nextLine),roomFile) != NULL ) {
  if ( nextLine[0] == '#' ) {
  } else if ( nextLine[0] == '@' ) {
  }
}

Mixing up fgetc and fgets lines just leads to confusion.

> roomInfo = realloc(roomInfo,(roomIndex+1)*sizeof(int)*ROOM_AT TRIBUTES);

Code:

void *t = realloc( old, size );
if ( t != NULL ) old = t; else doError();

First you need to handle the realloc error condition better. If it returns NULL, then your old pointer is still valid. If you immediately overwrite it with the return NULL result, you have a leak.
Then as noted for malloc, the new space is uninitialised.

I appreciate your responses and will definitely be implementing some of them after I fix my problem. I had the program print out the array after it was done running, and this is what I got:

Code:

1 4 0 0 2 0 0 0
2 420 1 0 3 0 4 0
3 813 2 0 0 0 0 0
4 1170 134481 5 0 6 0 2
5 1344 134449 0 0 4 0 0
6 1398 134417 4 0 7 0 0
7 1493 134385 0 0 0 0 0
8 1547 134353 0 0 0 0 0
9 1564 134321 0 0 0 0 0
10 1582 134289 0 0 0 0 0
11 1600 134257 0 0 0 0 0
12 1618 134225 0 0 0 0 0
13 1636 134193 0 0 0 0 0
14 1654 134161 0 0 0 0 0
15 1672 134129 0 0 0 0 0
16 1690 134097 0 0 0 0 0
17 1708 134065 0 0 0 0 0
18 1726 134033 0 0 0 0 0
19 1744 134001 0 0 0 0 0

Except for the third column, everything is working just fine (rooms 7 - 19 have no exits yet). The values in the third column appear to be memory addresses or something, since they each decrement themselves by 32. What's also weird is that they don't show up until the fourth iteration. Any ideas on what could be going on and how I can fix it?

Without your code, we can't possibly guess what you did wrong.

The code is right up there in the first post. I just stuck a bunch of "printf" statements right after it to see what was going on. The columns in that data correspond directly to how I described them in the comment on the code. I'm not sure what other code you could be wanting.

I did try using "memset" to set all of the values following my initial array to zero, though that didn't change the output that I posted above. I would post my code for that bit, but right now it seems the server where my code is at is down. (I've been doing this on a school computer, since I don't have Linux at home or the know-how to do this on my Windows machine.) When I get a chance I'll post that much if you want it.

Regardless, I appreciate the help you've given so far. If you have even the vaguest inkling as to what might be going on, feel free to mention it. In the meantime, I'll continue to fiddle with the code and scratch my head.

Ooh! I just remembered something. Previously, I had made this array an array of longs rather than ints. I changed it, because I expected this array to get fairly large. When it was longs, though, the output looked just fine.

> Previously, I had made this array an array of longs rather than ints.
And did you change all your scanf "%d" to "%ld" when you did that?


Your code, with a very simple main() to call it.
And a single line to make sure the allocated memory has known 'junk'.

Code:

$ cat foo.c
#include <stdio.h>
#include <string.h>
#include <stdlib.h>

/* Each of the rooms is a row of the array
and each room attribute is a column of the array */
/* Room attributes:
 * column 0 Room Number
 * column 1 Location of room in room file (character offset)
 * column 2 Number of the room to the north
 * column 3 Number of the room to the east
 * column 4 "   "   "   " south
 * column 5 "   "   "   " west
 * column 6 "   "   "   " up
 * column 7 "   "   "   " down
 */

#define ROOM_ATTRIBUTES 8

/* The pointer to the room array needs to be passed to other functions,
 * so this typedef is made in the header file
 */
typedef int (*rmArray)[ROOM_ATTRIBUTES];

rmArray Parse_Room_List(FILE *roomFile) {
        int nextChar;
        char nextLine[100];
        int roomIndex = 0;

    /* This is supposed to initialize the size of the room array.
     * I expect my problem may be here.
     */
        rmArray roomInfo = malloc(sizeof(int)*ROOM_ATTRIBUTES);
        for(;;) {
                nextChar = fgetc(roomFile);

        /* If the next character is a "#" character, then the number that follows
         * is a room number
         */
                if (nextChar == '#') {

            /* This resizes the array based on the number of rooms logged.
             * I expect I may have a problem here too.
             */
                        roomInfo = realloc(roomInfo,(roomIndex+1)*sizeof(int)*ROOM_ATTRIBUTES);
/*!! fake the garbage */{ int i; for ( i = 0 ; i < ROOM_ATTRIBUTES ; i++) roomInfo[roomIndex][i] = -1; }

            /* Gets the rest of the line, extracts the number, and stores it in
             * the 0th attribute column for the current room index.
             */
                        fgets(nextLine,sizeof(nextLine),roomFile);
                        sscanf(nextLine,"%d",&roomInfo[roomIndex][0]);

            /* Gets the current position in the file and stores it in the 1st
             * attribute column.
             */
                        roomInfo[roomIndex][1]=(int) ftell(roomFile);
                        roomIndex++;
                }

        /* If the next character is a "@" character, then the number that follows
         * is an attribue for the most recent room
         */
                else if (nextChar == '@') {
                        roomIndex--;

            /* nextChar stores the room attribute letter (n, e, s, w, u, or d)
             * and nextLine stores the rest of the line following that character
             */
                        nextChar = fgetc(roomFile);
                        fgets(nextLine,sizeof(nextLine),roomFile);

            /* Depending on which character was read, the number in the line
             * is extracted and stored in the appropriate attribute column.
             */
                        switch(nextChar){
                                case 'n':
                                        sscanf(nextLine,"%d",&roomInfo[roomIndex][2]);
                                        break;
                                case 'e':
                                        sscanf(nextLine,"%d",&roomInfo[roomIndex][3]);
                                        break;
                                case 's':
                                        sscanf(nextLine,"%d",&roomInfo[roomIndex][4]);
                                        break;
                                case 'w':
                                        sscanf(nextLine,"%d",&roomInfo[roomIndex][5]);
                                        break;
                                case 'u':
                                        sscanf(nextLine,"%d",&roomInfo[roomIndex][6]);
                                        break;
                                case 'd':
                                        sscanf(nextLine,"%d",&roomInfo[roomIndex][7]);
                                        break;
                        }
                        roomIndex++;
                }
                else if (nextChar == EOF)
                        break;
        }
        return roomInfo;
}

int main ( ) {
  FILE *fp = fopen("file.txt","r");
  rmArray   r = Parse_Room_List(fp);
  fclose(fp);
  for ( int i = 0 ; i < 5 ; i++ ) {
    for ( int j = 0 ; j < ROOM_ATTRIBUTES ; j++ ) {
      printf("%d ", r[i][j] );
    }
    printf("\n");
  }
  free(r);
  return 0;
}

Your short data file

Code:

$ cat file.txt
#1
@n5
#2
@e5
#3
@s5
#4
@w5
#5
@n3
@e4
@s1
@w2

Compile and run.
Note that all the output is as expected. Everything that isn't in the file is -1.
There are no bad memory accesses, and no leaks.

Code:

$ gcc -std=c99 -W -Wall foo.c
$ valgrind ./a.out 
==3433== Memcheck, a memory error detector.
==3433== Copyright (C) 2002-2008, and GNU GPL'd, by Julian Seward et al.
==3433== Using LibVEX rev 1884, a library for dynamic binary translation.
==3433== Copyright (C) 2004-2008, and GNU GPL'd, by OpenWorks LLP.
==3433== Using valgrind-3.4.1-Debian, a dynamic binary instrumentation framework.
==3433== Copyright (C) 2000-2008, and GNU GPL'd, by Julian Seward et al.
==3433== For more details, rerun with: -v
==3433== 
1 3 5 -1 -1 -1 -1 -1 
2 10 -1 5 -1 -1 -1 -1 
3 17 -1 -1 5 -1 -1 -1 
4 24 -1 -1 -1 5 -1 -1 
5 31 3 4 1 2 -1 -1 
==3433== 
==3433== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 11 from 1)
==3433== malloc/free: in use at exit: 0 bytes in 0 blocks.
==3433== malloc/free: 7 allocs, 7 frees, 864 bytes allocated.
==3433== For counts of detected errors, rerun with: -v
==3433== All heap blocks were freed -- no leaks are possible.
$

Originally Posted by Salem

Code:

/*!! fake the garbage */{ int i; for ( i = 0 ; i < ROOM_ATTRIBUTES ; i++) roomInfo[roomIndex][i] = -1; }

This "fake the garbage" line that you made is actually solving the problem, so I've got the code working now. I think my problem had to do with me not initializing the memory after I reallocated it; I'm not sure why I was consistently getting the same type of result, but it looks like after about the fourth iteration, the address of the array was changed, and all the old pointers got read into the array. Thanks for all the help.