0

I am relatively new to programming in general, and I'm trying to write some code to work with square matrices. Unfortunately, I'm stuck very early in the development, as the code

typedef struct Matrix_t{
    float** content;
    size_t size;
} Matrix_t;

int main(int argc, char** argv) {
    Matrix_t* matr;

    initMatrix(matr,s);
    /*
    ...
    ...
    ...
    */
    return 0;
}

void initMatrix(Matrix_t* m, size_t s) {
    int i;

    m = (Matrix_t*) malloc(sizeof(Matrix_t));
    m->content = (float**) malloc(s*sizeof(float*));
    for(i=0;i<s;i++){
        m->content[i] = (float*) malloc(s*sizeof(float));
    }

    m->size = s;
}

would SIGSEGV immediately after initMatrix() is done. Using the debugger, I found out that basically all matrix info is lost after initMatrix() is closed. Why is it? And how can I fix it?

Thanks in advance.

Luigi D.
  • 165
  • 1
  • 10
  • 1
    `m = ...` if you think this will affect the pointer passed in from `main()`, you may want to rethink that. Further, I see no reason to dynamically allocate `Matrix_t` in the first place. Its relevant *content* is already dynamic-managed. What are you gaining but a few more bytes on the heap? – WhozCraig Jan 15 '15 at 19:48
  • Variable `m` is a local variable in function `initMatrix`. Anything you set it to is irrelevant outside the function. In essence, the allocated memory is "lost" (a.k.a. *memory leak*) once this function completes. – barak manos Jan 15 '15 at 19:51
  • @barakmanos `m` is local but it's actually the `matr` variable's address. You pass a Matrix pointer to the function, function pointer points to allocated memory, after return of `initMatrix`, `m` should still have the allocated memory address. Am I wrong? It's weird that code doesn't work. What do you think? – Mustafa Chelik Jan 15 '15 at 20:12
  • Ah! Address of pointer should be passed. I got it thanks to WhozCraig – Mustafa Chelik Jan 15 '15 at 20:16
  • I suggest avoiding the phrase "double pointer". It could refer to a pointer to a pointer, as it does here, but it also refers to the type `double*`. Click on the "double-pointer" tag, which takes you to the tag wiki that discusses this issue. – Keith Thompson Jan 15 '15 at 20:17
  • @MustafaChelik: Yes, you are wrong. OP is passing **the value** of `matr`, not the address (and the function doesn't take a `Matrix_t**` argument anyway, so if OP passed the address of that variable, then the code wouldn't even compile). – barak manos Jan 15 '15 at 20:44
  • a few suggestions: 1) eliminate the typedef and simply define a struct type. (note: references to that type will then need to be prefixed with 'struct' 2) add the missing prototype for the 'initMatrix' function 3) always check the returned value from a call to malloc() to assure the operation was successful. 4) change this line: 'initMatrix(matr,s);' (and associated code) to: 'initMatrix(&matr,s);' so can change the actual contents of the matr variable. 5) for several reasons, do not cast the returned value from a call to malloc 6) always set pointers to an initial value when defined – user3629249 Jan 15 '15 at 22:17
  • Consider using a contiguous memory bloc for the matrix , instead of a jagged array. – M.M Jan 16 '15 at 00:32

2 Answers2

1

You're modifying only a local automatic variable m in your function. C is a pass-by-value language. If you want to pass by address, then an address is what you have to pass, declaring the formal parameter as a pointer-to-type, even when that type is already a pointer and what you're modifying is the the pointer itself.

When you're passing something as an in/out or out parameter, If you find yourself doing this:

void foo(Type *p)
{
    p = malloc(...)
}

you're not modifying the data pointed to by p, you're modifying p itself, and the caller will be unaware of the change. The address stored in p upon entrance is lost. Where as this:

void foo(Type *p)
{
    *p = ....
}

is modifying what p points to. That said, if Type is already a pointer-type, and what you want to modify it the pointer itself, you must do this:

void foo(Type **pp) // declare a pointer-to-pointer-to-type
{
    *pp = malloc(....) // dereference to store the address returned from malloc
                       // in the pointer pointed to by pp
}

Therefore, the most immediate fix is to declare the formal parameter as a pointer-to-pointer-to-type, passing the address of matr from main()

void initMatrix(Matrix_t **pp, size_t s) 
{
    int i;

    Matrix_t *m = malloc(sizeof(Matrix_t));
    m->content = malloc(s*sizeof(float*));
    for(i=0;i<s;i++)
        m->content[i] = malloc(s*sizeof(float));
    m->size = s;
    *pp = m; // note: saving pointer to the output target
}


int main(int argc, char** argv) 
{
    Matrix_t* matr = NULL;

    initMatrix(&matr,s); // note: passing address of pointer

    /*
    ...
    ...
    ...
    */
    return 0;
}

I left out the error checking (or I should say, I didn't add any, as there was none there to begin with) and removed the unneeded casts from malloc, which you can read more about here.

There are about a half-dozen ways to do this, but this is the most direct to what code you already have. I would advise either returning the allocation from the function itself as the ret-val, or not even dynamic-allocating the structure itself as there is no need to do so. Both of which I leave for you to think about.

Best of luck.

Community
  • 1
  • 1
WhozCraig
  • 65,258
  • 11
  • 75
  • 141
1

Make matr=NULL in main function and so, try:

main(){

   Matrix_t *matr=NULL;
   /*Declare and initialize "s" variable.... */

   initMatrix(&matr,s);  /* See below...  */
   /*
     .....
   */
}

Use a pointer to pointer in Matrix_t *m in function initMatrix

void initMatrix(Matrix_t **m, size_t s) 
{    
     Matrix_t *aux=(Matrix_t*)malloc(sizeof(Matrix_t));

     /*check if matrix already allocated */  
     if(*m != NULL) 
     {    
         /*Make a free matrix function to free the matrix before use*/          
         free_my_matrix(m); 
         *m=NULL;
     }

     /***
        Work with aux....
        aux->content=...malloc....
     ***/

     /* make your base pointer be aux */           
     *m=aux; 
}