Pointers problem

Hi there. I'm new to the forum and this is my first time posting here. I'm a newbie at C and I'm experiencing a pointers problem. Here's the code:

Code:

#include <stdio.h>

float *mult_matrix(int n, float *p, float *q);
float *get_matrix(int m, int n);
void show_matrix(int m, int n, float *p);

int main(void)
{
    int n;
    float *a, *b, *x;

    printf("\nEnter the size of the linear system: \n");
    scanf("%d", &n);
    
    a=get_matrix(n, n);
    show_matrix(n, n, a);
    b=get_matrix(n, 1);
    show_matrix(n, 1, b);
    x=mult_matrix(n, a, b);
    show_matrix(n, 1, x);

    return 0;
}

float *mult_matrix(int n, float *p, float *q)
{
    int i, j;
    float X[n];
    float *x;    
    
    for(i=0; i<n; i++) {
        X[i]=0;
    }

    for (i=0; i<n; i++) {
        for(j=0; j<n; j++) {
            X[i]=X[i]+(*p)*(*q);
            p++;
            q++;
        }    
        q=q-n;
    }
    x=X;

    return x;
}

float *get_matrix(int m, int n)
{
    int i, j;
    float *a, A[m][n];
    a=&A[0][0];

    for (i=0; i<m; i++) {
        for (j=0; j<n; j++) {
            printf("\nEnter with the element a%d%d of the matrix:", i+1, j+1);
            scanf("%f", a);
            a++;
        }    
    }

    a=a-m*n;

    return a;
}

void show_matrix(int m, int n, float *p)
{
    int i, j;
    
    printf("\n");
    for (i=0; i<m; i++) {
    printf("\n");
        for (j=0; j<n; j++) {
            printf(" %f ", *p);
            p++;
        }
    }
    printf("\n");
}

It is supposed to be a program that receives a matrix and a vector and multiplies them. It compiles, but when it goes to the function Show_Matrix it shows completely different values, so the result is wrong. I've tried Splint debugger so I go the folling messages:

Splint 3.1.2 --- 03 May 2009

matrix.c: (in function main)
matrix.c:13:2: Return value (type int) ignored: scanf("%d", &n)
Result returned by function call is not used. If this is intended, can cast
result to (void) to eliminate message. (Use -retvalint to inhibit warning)
matrix.c:22:11: Fresh storage a not released before return
A memory leak has been detected. Storage allocated locally is not released
before the last reference to it is lost. (Use -mustfreefresh to inhibit
warning)
matrix.c:15:2: Fresh storage a created
matrix.c:22:11: Fresh storage b not released before return
matrix.c:17:2: Fresh storage b created
matrix.c:22:11: Fresh storage x not released before return
matrix.c:19:2: Fresh storage x created
matrix.c: (in function mult_matrix)
matrix.c:37:9: Value X[] used before definition
An rvalue is used that may not be initialized to a value on some execution
path. (Use -usedef to inhibit warning)
matrix.c:45:9: Stack-allocated storage x reachable from return value: x
A stack reference is pointed to by an external reference when the function
returns. The stack-allocated storage is destroyed after the call, leaving a
dangling reference. (Use -stackref to inhibit warning)
matrix.c:43:2: Storage x becomes stack-allocated storage
matrix.c:45:9: Returned storage x not completely defined (*x is undefined): x
Storage derivable from a parameter, return value or global is not defined.
Use /*@out@*/ to denote passed or returned storage which need not be defined.
(Use -compdef to inhibit warning)
matrix.c:43:2: Storage *x allocated
matrix.c: (in function get_matrix)
matrix.c:57:4: Return value (type int) ignored: scanf("%f", a)
matrix.c:64:9: Returned storage a not completely defined (*a is undefined): a
matrix.c:52:2: Storage *a allocated
matrix.c:3:8: Function exported but not used outside matrix: mult_matrix
A declaration is exported, but not used outside this module. Declaration can
use static qualifier. (Use -exportlocal to inhibit warning)
matrix.c:46:1: Definition of mult_matrix
matrix.c:4:8: Function exported but not used outside matrix: get_matrix
matrix.c:65:1: Definition of get_matrix
matrix.c:5:6: Function exported but not used outside matrix: show_matrix
matrix.c:80:1: Definition of show_matrix

Finished checking --- 12 code warnings

Please, could somebody help me and tell me what's going on? Cheers.

I'm going to ignore what splint says. That's a pretty bad idea, but you don't know how to use it and I don't have time to teach you so there is no point in talking about it yet.

In general there is a lot of bad stuff going on. Your biggest problem is stuff like this:

Code:

    a=get_matrix(n, n);
...
float *get_matrix(int m, int n)
{
    int i, j;
    float *a, A[m][n];
    a=&A[0][0];
 
    for (i=0; i<m; i++) {
        for (j=0; j<n; j++) {
            printf("\nEnter with the element a%d%d of the matrix:", i+1, j+1);
            scanf("%f", a);
            a++;
        }   
    }
 
    a=a-m*n;
 
    return a;
}

When this function returns, the pointer a is invalid, because the matrix A has fallen out of scope. In other words, a is only valid inside of the function get_matrix. You repeat this mistake in the other function, mult matrix, with the X array and the x pointer. This is undefined behavior, which makes your program unpredictable. It could appear to work on your computer in spite of everything, and break terribly on another computer.

The appropriate way of handling memory is to allocate it at the start of a function or block, and free it at the end of the block. It looks like to me that you want to use A and X throughout the program, so these variables should be allocated in main, and then passed in as arguments to the other functions.

Originally Posted by whiteflags

The appropriate way of handling memory is to allocate it at the start of a function or block, and free it at the end of the block. It looks like to me that you want to use A and X throughout the program, so these variables should be allocated in main, and then passed in as arguments to the other functions.

Allocating these blocks inside `get_matrix()` and `mult_matrix()`, and then returning a pointer to them would likewise work.

Originally Posted by msh

Allocating these blocks inside `get_matrix()` and `mult_matrix()`, and then returning a pointer to them would likewise work.

Not the way the program is being written right now, no. That actually would make things look completely different.

Originally Posted by whiteflags

Not the way the program is being written right now, no. That actually would make things look completely different.

Some changes would be necessary with either approach. Granted, I think what you propose would actually be easier to do, given the code already written. Touche.

Originally Posted by whiteflags

I'm going to ignore what splint says. That's a pretty bad idea, but you don't know how to use it and I don't have time to teach you so there is no point in talking about it yet.

In general there is a lot of bad stuff going on. Your biggest problem is stuff like this:

Code:

    a=get_matrix(n, n);
...
float *get_matrix(int m, int n)
{
    int i, j;
    float *a, A[m][n];
    a=&A[0][0];
 
    for (i=0; i<m; i++) {
        for (j=0; j<n; j++) {
            printf("\nEnter with the element a%d%d of the matrix:", i+1, j+1);
            scanf("%f", a);
            a++;
        }   
    }
 
    a=a-m*n;
 
    return a;
}

When this function returns, the pointer a is invalid, because the matrix A has fallen out of scope. In other words, a is only valid inside of the function get_matrix. You repeat this mistake in the other function, mult matrix, with the X array and the x pointer. This is undefined behavior, which makes your program unpredictable. It could appear to work on your computer in spite of everything, and break terribly on another computer.

The appropriate way of handling memory is to allocate it at the start of a function or block, and free it at the end of the block. It looks like to me that you want to use A and X throughout the program, so these variables should be allocated in main, and then passed in as arguments to the other functions.

So you suggest to use the functions of dynamic allocation of memory on function main()? On this case, malloc() and then free()?

Originally Posted by guitarman

So you suggest to use the functions of dynamic allocation of memory on function main()? On this case, malloc() and then free()?

That, or you could put A and X on the stack in main(). You don't have to call free() on stack variables. Pick whatever you are the most comfortable with.

Originally Posted by whiteflags

That, or you could put A and X on the stack in main(). You don't have to call free() on stack variables. Pick whatever you are the most comfortable with.

So, I've tried putting a, x and b on the stack in main() but I'm still having trouble. Please, what I'm doing wrong?

Code:

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

float *mult_matrix(int n, float *p, float *q);
float *get_matrix(int m, int n);
void show_matrix(int m, int n, float *p);

int main(void)
{
    int n;
    float *a, *b, *x;

    printf("\nEnter the size of the linear system: \n");
    scanf("%d", &n);

    a=malloc((n*n)*sizeof(float));
    b=malloc((n)*sizeof(float));
    x=malloc((n)*sizeof(float));

    if ((!a)||(!b)||(!x)) {
        printf("\nError in memory solicitation.\n");
        exit(1);
    }    
    
    a=get_matrix(n, n);
    show_matrix(n, n, a);
    b=get_matrix(n, 1);
    show_matrix(n, 1, b);
    x=mult_matrix(n, a, b);
    show_matrix(n, 1, x);

    return 0;
}

Here's my analysis.

Code:

int main(void)
{
    int n;
    float *a, *b, *x;
 
    printf("\nEnter the size of the linear system: \n");
    scanf("%d", &n);
 
    a=malloc((n*n)*sizeof(float));
    b=malloc((n)*sizeof(float));
    x=malloc((n)*sizeof(float)); /* good so far ... */

    a=get_matrix(n, n); /* what happens in here? */
    show_matrix(n, n, a);
    b=get_matrix(n, 1); /* here too? */

OK so, assuming that get_matrix() hasn't changed, we have:

Code:

float *get_matrix(int m, int n)
{
    int i, j;
    float *a, A[m][n]; /* A is a variable length array from C99 */
    a=&A[0][0]; /* a doesn't point to the memory you allocated in main() anymore */
 
    for (i=0; i<m; i++) {
        for (j=0; j<n; j++) {
            printf("\nEnter with the element a%d%d of the matrix:", i+1, j+1);
            scanf("%f", a);
            a++;
        }   
    }
 
    a=a-m*n; /* odd, but acceptable */
 
    return a; /* the same undefined behavior from earlier */
}

Let's look at what happens to x.

Code:

float *mult_matrix(int n, float *p, float *q)
{
    int i, j;
    float X[n]; /* variable length array again */
    float *x;   
     
    for(i=0; i<n; i++) {
        X[i]=0;
    }
 
    for (i=0; i<n; i++) {
        for(j=0; j<n; j++) {
            X[i]=X[i]+(*p)*(*q);
            p++;
            q++;
        }   
        q=q-n; /* odd, but acceptable. */
    }
    x=X; /* memory leak here, x doesn't point to what you malloced anymore. */
 
    return x; /* the same undefined behavior from earlier */
}

You can't use malloc AND variable length arrays at the same time. It causes the mistakes I noted. Returning a pointer to an array on the stack will just lead to invalid pointers after the array falls out of scope. Trying to use malloc and variable length arrays at the same time will just cause memory leaks. You really need to pick one or the other. Once you fix what's wrong in these functions you should be able to call them as many times as you want to.

Finally, you might be asking why I think show_matrix is OK.

Code:

void show_matrix(int m, int n, float *p)
{
    int i, j;
     
    printf("\n");
    for (i=0; i<m; i++) {
    printf("\n");
        for (j=0; j<n; j++) {
            printf(" %f ", *p);
            p++;
        }
    }
    printf("\n");
}

It's all OK because you don't do any assignments to p. As long as p points to an array of integers, you will see them.

I realize this is confusing, but you probably lack a good lesson on what pointers do, so look here before you try to fix it again. See if it clears anything up for you. Binky Pointer Fun Video

Thanks. It was a great help. Here's the new code:

Code:

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

float *mult_matrix(int n, float *p, float *q);
float *get_matrix(int m, int n);
void show_matrix(int m, int n, float *p);

int main(void)
{
    int n;
    float *a, *b, *x;

    printf("\nEnter the size of the linear system: \n");
    scanf("%d", &n);

    a=get_matrix(n, n);
    show_matrix(n, n, a);
    b=get_matrix(n, 1);
    show_matrix(n, 1, b);
    x=mult_matrix(n, a, b);
    show_matrix(n, 1, x);

    return 0;
}

float *mult_matrix(int n, float *p, float *q)
{
    int i, j;
    float *x;

    x=malloc((n)*sizeof(float));    

    for(i=0; i<n; i++) {
        *x=0;
        x++;
    }
    x=x-n;

    for (i=0; i<n; i++) {
        for(j=0; j<n; j++) {
            *x=*x+(*p)*(*q);
            p++;
            q++;
            x++;
        }    
        q=q-n;
    }
    x=x-n;

    return x;
}

float *get_matrix(int m, int n)
{
    int i, j;
    float *a;
    
    a=malloc((m*n)*sizeof(float));

    for (i=0; i<m; i++) {
        for (j=0; j<n; j++) {
            printf("\nEnter with the element a%d%d of the matrix:", i+1, j+1);
            scanf("%f", a);
            a++;
        }    
    }

    a=a-m*n;

    return a;
}

void show_matrix(int m, int n, float *p)
{
    int i, j;
    
    printf("\n");
    for (i=0; i<m; i++) {
    printf("\n");
        for (j=0; j<n; j++) {
            printf(" %f ", *p);
            p++;
        }
    }
    printf("\n");
}

It looks that it is working well, but I have some doubts. Do I have to use free()? What happens to the memory allocated inside functions?

You should be free()ing what you malloc()

Originally Posted by Syscal

You should be free()ing what you malloc()

But main() doesnt free it when it returns?

Originally Posted by guitarman

But main() doesnt free it when it returns?

People say that modern operating systems can do that, but it's better just to write complete code. I mean you're depending on that feature. OTOH, free() is standardized.