Tricky pointer problem in C

[willm07]

Hey Everyone

I am having some trouble with pointers in my C program. I vaguely know how they work and I have read countless (very good) tutorials, such as the ones here:

http://www.google.com/search?q=c+pointers

.. but I can't get my head around what is going wrong with my program.

I am writing a program to store a sparse matrix. My datastructures are:

struct MList {

	int index;

	int value;

	struct MList *next;

} MList;

typedef struct Matrix {
    
    int nRows;
    
    int nCols;
    
    MList **rows;
    
    MList **cols;
    
} Matrix;

Matrix is an array of MLists (linked lists). Each element in the array corresponds to either a row or a column of the matrix.

The Matrix structure holds two arrays of MLists. Each hold identical data, but having one by row and one by column makes some operations (such as transpose) a lot easier.

I hope I haven't lost you..

Anyway, I have a function to insert a matrix from a file into the structure described above, which works fine (and I have thoroughly tested it). The problem is, I also have a function to print a matrix from the structure (by looping through each element in the array and trawling through each respective linked list) to stdout which returns "Segmentation Fault".

I have isolated the code causing this fault and it is within the second WHILE loop. But I don't understand why it is causing this error.

int SaveToFile(char *filename, Matrix *m)
{
    
    FILE *FP;
    int i=0;
    
    FP = fopen(filename, "w");
    
   // loop through every row in the matrix data structure 
   while(m->nRows > i)
    {
        
        printf("Running\n");
        
        // for each row, print every entry

        // create a pointer to the first element in the linked list in the current row
        MList * currentEl = m->rows[i];
        
        while(currentEl != NULL)
        {
            // print the element in the linked list to std out
            printf("%d %d %d \n", i, currentEl->index, currentEl->value);
            
            currentEl = currentEl->next;
            
        }
        
        i++;
    }
    
    return 1;
}

Output:

Running
0 1769107551 6714478 
Running
1 1969905503 1952411757 
Segmentation fault

I know this is probably trivial for some experts out there.. does anyone have an idea what is going wrong?

Also, why are

currentEl->index
currentEl->value

both printing out long numbers (memory addresses?) instead of the values I assigned to them?

Any help you can give me would be MUCH appreciated, I have been stuck on this all day and it is driving me insane

[Sebastiani]

you should probably post the MList allocation/initialization code.

[willm07]

you should probably post the MList allocation/initialization code.

Thanks for the quick reply.

OK.. this is the function that creates the structure from a text file; outputs a pointer to a Matrix structure:

Matrix *CreateFromFile(char *filename)
{
        // variable declarations
        Matrix *m;
    
        FILE *FP;
    
        char snRows[50], snCols[50];
        int nRows, nCols;
    
        char srow[50], scol[50], sdata[50];
        int rnum, cnum, data;
    
        // allocate space in memory for m
        m = (Matrix *) malloc(sizeof(Matrix));
    
        FP = fopen(filename, "r");
    
        // read in number of rows and columns intially, and convert to int
        fscanf(FP, "%s %s", snRows, snCols);

        m->nRows = atoi(snRows);
        m->nCols = atoi(snCols);
        
        // Initialise arrays of MList
        struct MList *rows[m->nRows];
        struct MList *cols[m->nCols];
        
        // process the file on a line-by-line basis
	while( fscanf(FP, "%s %s %s", srow, scol, sdata) != EOF )
        
	{
            
            rnum = atoi(srow);
            cnum = atoi(scol);
            data = atoi(sdata);
            
            rows[rnum] = list_insert(cnum, data, rows[rnum]);
            cols[cnum] = list_insert(rnum, data, cols[cnum]);
        }
        
        
        m->rows = rows;
        m->cols = cols;
        
        
        return m;
    
}

and the function list_insert()

MList *list_insert( int index, int value , MList *pl ) {
	
	MList *t = calloc( 1, sizeof( MList ) );
	
	t->index = index;

	t->value = value;

	// Set pointer to previous list
	t->next = pl;

	return t;

}

(this function outputs a pointer to a new MList structure)

The number of columns and rows are specified in the first line of the input file (supplied to this function as filename)

[foxman]

    // Initialise arrays of MList
    struct MList *rows[m->nRows];
    struct MList *cols[m->nCols];

This is incorrect. It shouldn't even compile if it was true C but from what i see you are compiling your code as C++, not C.

Anyway. You need to declare your arrays on the heap (by using malloc or calloc) instead of the stack. How can you do this ? Well...

    m->rows = malloc(m->nRows * sizeof(struct Mlist *));  // or malloc(m->rows * sizeof(*(m->rows)));
    m->cols = malloc(m->nCols * sizeof(struct Mlist *));
    if (m->rows == NULL || m->cols)
        // Memory allocation failed, return an error code or exit program

Should resolve some of the problems. Also, i would suggest you to read every line from the file using fgets() and then reading the value in the string with sscanf(). And no need to used atoi(). I'll let you figure this out.

By the way you should check what malloc (or calloc) and fopen returns.

[willm07]

foxman,

Thank you very much for your help, that has fixed it. I think it was that before I was creating a pointer to a pointer to a pointer:

(ie. *rows[] was an array of pointers to MLists and then with this line:

m->rows = rows;
m->cols = cols;

I was creating a pointer to a pointer to the array of pointers, which is not allowed in C (am I right? The most chaining you can do is to have a pointer to a pointer)

Now I am allocating memory directly at the pointed location of m->rows.

Not sure what you meant about heap and stack, I thought that all data is stored on the heap in C?

Also, is the only c++ syntax I am using with //s instead of /**/ for comments..?

Thanks for the hint about fgets() and sscanf()

[Elysia]

No, you can have as many pointers to pointers to pointers to arrays, etc, as you want. The problem is that you can't initialize an array with a non-constant value unless you use dynamic memory functions.

int n = 5;
int MyArray[n]; // Illegal
int* MyArray = malloc(n * sizeof(int)); // Works

Also /* */ are for C. // is for the C99 standard and C++.

[MacGyver]

You can dynamically allocate arrays with non-constant values (ie. variables) in C99. I would personally discourage this practice, since blasting the stack to pieces with a large array isn't always handled gracefully.

[willm07]

No, you can have as many pointers to pointers to pointers to arrays, etc, as you want. The problem is that you can't initialize an array with a non-constant value unless you use dynamic memory functions.

int n = 5;
int MyArray[n]; // Illegal
int* MyArray = malloc(n * sizeof(int)); // Works

Also /* */ are for C. // is for the C99 standard and C++.

Then how come the following code works?

char snRows[50], snCols[50];
fscanf(FP, "%s %s", snRows, snCols);

Also.. are all variables in C stored on the stack unless otherwise specified with malloc/calloc etc.?

[matsp]

Then how come the following code works?

char snRows[50], snCols[50];
fscanf(FP, "%s %s", snRows, snCols);

Also.. are all variables in C stored on the stack unless otherwise specified with malloc/calloc etc.?

All local variables [variables inside functions, along with parameters to functions] are stored on the stack, unless specifically declared "static" [which essentially makes it a global variable, but not visible to other functions].

Actually, the code you quote is by no means sure to work. If there is less than 100 bytes[1] available on the stack when this code is called, then the program will crash, and it's quite possible that the application can't recover from this [because you've run into the HARD limit of the stack-size, the stack grows dynamically until you hit the limit]. Of course, fscanf() will also use some amount of stack-space.

With a malloc (etc) scheme of memory management, you at least have SOME chance of recovering from the situation with some sort of meaningfull error message - even if the application can't continue, you will be able to tell your user that the problem was "out of memory", rather than whatever the system says if you ran out of stack.


[1] Because of alignment, local registers and various other things, a function may need more than the apparent amount of stack-space. The exact amount can only really be found by examining the generated code, and may of course change if you modify the compiler options, new version of compiler or some such.

--
Mats

[Elysia]

fscanf(FP, "%s %s", snRows, snCols);

Read this:
http://cboard.cprogramming.com/showp...37&postcount=9

[ElemenT.usha]

MATSP running out of stack..? i really dont get the point here...
do you meAn to say that

int array[20];
for (i=0;i<20;i++)
scanf("&#37;d",&array[i]);

wil crash if i do not have 20 bytes of space o the stack... if yes then how does malloc allocate the space...

then do we need for memory allocation .... checking the stack everytime...

[Elysia]

Matsp is saying IF there's not enough space. If you've sucked up all space previously, then YES, it will crash with when you define that array, otherwise NO, it won't crash.
The stack is a special place in memory that's limited to a certain size. Malloc allocates memory from the heap, which is located in the computer RAM (which we typically have 1-2 GB of).
However, the STACK is limited to about 1 MB or so.

[ElemenT.usha]

so does the stack clean up everytime he program is closed????

wat would be the error notice if stack overrun ccurs...
can we check the memory available in a stack???

[Elysia]

All variables on the stack are automatically cleaned up when the function exits.
Yes, you would notice if a stack overrun occurs--your app crashes!
No, there's no portable way of doing that. But otherwise, sure, if you use assembly, you can.

[vart]

All variables on the stack are automatically cleaned up when the function exits.

The wording is not too good. The data stays there... till the new data is places over it. So no automatic "clean up" occures... If you access the local var after you exit function - you can by luck get the old value... or some garbage value, if this address is already used for something else by the new function...

by clean up, of course, you can understand the automatic calling of destructor, but it happens only in C++ for objects...