Dynamic deallocation of a 2D matrix crashing

Question

I have a function which allocates 2D matrix at the start and a function which deallocates it, which I use at the end.

int** CreatMat(int N){
    int i,**T;
    T = (int**)malloc(sizeof(int*)*N);
    if(T!=NULL){
        for(i=0;i<N;i++){
            T[i]=(int*)malloc(sizeof(int)*N);
            if(T[i]==NULL){
                printf("\nCreatMat()::Allocation failed at block %d",i);
                for(i=i;i>=0;i--){
                    free(T[i]);
                    T[i]=NULL;
                }
                free(T);
                T=NULL;
                return T;
            }
        }
    }
    return T;
}

//Free a dynamic matrix.
void FreeMat(int** T,int N){
    int i;
    for(i=0;i<N;i++){
        free(T[i]);
        T[i]=NULL;
    }
    free(T);
    T = NULL;
}

Somehow, FreeMat() is crashing. any help?

Full code here

~janky fix code here

other functions that edit the ints in those matrices, and prints them, I wanted to keep it short here, I can add them if u want. — Yacine Megrah, Jan 10 '22 at 23:10
This code is fine. The problem is in the code you haven't shown us. Presumably you're stepping on memory you shouldn't, so run your code through valgrind to find the problem. — dbush, Jan 10 '22 at 23:12
Welcome! Please post a [Minimal Reproducible Example](https://stackoverflow.com/help/minimal-reproducible-example) as text, the shortest *complete* code that shows the fault (with the runtime input). The best way to do that is by copy/paste, after you check it does exhibit the behaviour described. — Weather Vane, Jan 10 '22 at 23:12
Not usually, it makes it uphill for readers. But: in `main()` this `if(Grids_Init(T,S)!=0)` does *not* affect the value of the local variables `S` and `T` which remain uninitialised and you then go on to free these indeterminate pointers. The deployment of a debugger would have revealed this quickly. — Weather Vane, Jan 10 '22 at 23:30
Keep it simple. Use one function to initialise one of them, `return` the pointer and assign it to `S`. Then same for `T`. — Weather Vane, Jan 10 '22 at 23:41
Please don't post answers inside the question. I've done a rollback of changes. If you wish to expand on the posted answers, it's perfectly fine to post an answer to your own question below. — Lundin, Jan 11 '22 at 14:16

score 0 · Accepted Answer · answered Jan 10 '22 at 23:52

In function main() this

int **T, **S;
if(Grids_Init(T, S) != 0)

does not affect the value of the local variables S and T which remain uninitialised, and you then go on to free these indeterminate pointers.

You can use a function to initialise one of them, return the pointer and assign it to T. Then same for S.

This is preferable to using the three-star pointers: please see Triple pointers in C: is it a matter of style? One answer begins with

Using triple pointers is harming both readability and maintainability.

score 0 · Answer 2 · answered Jan 11 '22 at 01:27

0

You do not create 2D array only an array of pointers. Make your life easier and locate a real 2D array. Also, use the correct type for sizes size_t

void CreatMat(size_t N, int (**array)[N])
{
    *array = malloc(N * sizeof(**array));
}

int main(void)
{
    int (*array)[N];
    CreatMat(1000, &array);

    /* some code */

    free(array);
}

See how much easier it is.

answered Jan 11 '22 at 01:27

0___________

60,014
4
34
74

Thank you, this would make things easier, but if we were to define a `array` type how would we define it? also using `sizeof()` and passing an `*******ptr` to it will always return 8bits, I think. – Yacine Megrah Jan 11 '22 at 11:02
@YacineMegrah I do not understand `*******ptr`. Your comment makes not too much sense – 0___________ Jan 11 '22 at 11:39
I meant that passing any pointer to `sizeof()` will always return 8; – Yacine Megrah Jan 11 '22 at 11:47
@YacineMegrah you do not understand what my code does. Test it first, and do not assume anything – 0___________ Jan 11 '22 at 11:49
how can I access a `T[i][j]` in `main()` ? is it `array[i][j]`? – Yacine Megrah Jan 11 '22 at 12:07
@YacineMegrah yes. – 0___________ Jan 11 '22 at 12:23
I used `void PrintMat(int N,int (*array)[N])` then called `PrintMat(7,array)` which prints every `array[i][j]`, could you tell me where is the faulty part in this? cuz it didn't print anything. – Yacine Megrah Jan 11 '22 at 12:30
@YacineMegrah I do not see `PrintMat` function in your question. – 0___________ Jan 11 '22 at 12:35
Stack Overflow should really add a dropdown code snips or smth, I am going ham on pastebin rn, this is your code – Yacine Megrah Jan 11 '22 at 12:40
@YacineMegrah Soply edit your question - no external links – 0___________ Jan 11 '22 at 12:55
I have added another code to question, Is it equal to yours? – Yacine Megrah Jan 11 '22 at 14:15
@YacineMegrah Check out [Correctly allocating multi-dimensional arrays](https://stackoverflow.com/questions/42094465/correctly-allocating-multi-dimensional-arrays). If you scroll down to the bottom of the answer, there's an example showing that the level of indirection using this method doesn't get worse than `**`, for example `void arr_alloc (size_t x, size_t y, size_t z, int(**aptr)[x][y][z])` would be used for a 3D array. – Lundin Jan 11 '22 at 14:19

Yacine Megrah · Answer 3 · 2022-01-11T15:07:15.963

0

In response to @0_______

#include <stdlib.h>

int main(){
    int i,j;
    int (*T)[7];
    //(*T)[7] = malloc(7*sizeof(&(*T))); is wrong
    T = malloc(7*sizeof(*T));
    for(i=0;i<7;i++){
        printf("\n");
        for(j=0;j<7;j++){
            printf("%d  ");
        }
    }
    free(T);

    return 0;
}

edited Jan 11 '22 at 15:07

answered Jan 11 '22 at 14:19

Yacine Megrah

5
3

You might find `T = malloc( sizeof(int[7][7]) );` less confusing and easier to read. – Lundin Jan 11 '22 at 14:21
@Lundin I would love to write that if I could, "Is it qual to `(*T)[7] = malloc(7*sizeof(&(*T)));` ?" is my concern. – Yacine Megrah Jan 11 '22 at 14:26
no it is wrong!!! if you want 7x7 `T = malloc(7*sizeof(*T));` – 0___________ Jan 11 '22 at 14:29
@Lundin it is error-prone. You need to change all sizeofs if you change the definition of `T`. The idea is to use objects not types in sizeof to make your code less error-prone – 0___________ Jan 11 '22 at 14:32
`&*` is the same as not using either operator so you end up with `sizeof(T)` rather than `sizeof(*T)` as intended. – Lundin Jan 11 '22 at 14:32
@0___________ As demonstrated by the OP, the `*T` version is equally error prone, or they would have gotten it right. It's subjective coding style which version to use. Obviously one should use named constants instead of magic numbers though. Better yet, wrap all of it in a struct. – Lundin Jan 11 '22 at 14:34
Sorcery! it worked! – Yacine Megrah Jan 11 '22 at 14:35
@Lundin , so you suggest `typedef struct {int(*M)[7]} matrix_7; `? and `typdef matrix int**` for Original iteration? – Yacine Megrah Jan 11 '22 at 14:45
@YacineMegrah never hide arrays and pointers behind the typedefs – 0___________ Jan 11 '22 at 14:46
@Lundin indeed - first you need to learn what `*` & `&` do. Without it everything is error prone – 0___________ Jan 11 '22 at 14:47
@YacineMegrah No I suggested that you would simply wrap the array inside a struct and leave allocation of it as a separate matter. That is `typedef struct { int m[7][7]; } matrix;`. Not very flexible though. But this is heading off-topic. – Lundin Jan 11 '22 at 14:49
Well, my understanding is that : `*` could be used to declare a pointer, or to dereference it (down a step or power) , and `&` is used to access the adresse of something (default pointer to that target) . – Yacine Megrah Jan 11 '22 at 14:56
`*` can be used for dereferencing and `&` for referencing, so they take each other out. The C standard explicitly states that this is what happens when you use `&*`. – Lundin Jan 11 '22 at 15:22

score 0 · Answer 4 · answered Jan 11 '22 at 14:34

representing a matrix as an array of pointers is suboptimal: it wastes memory and time, and the locality of reference will be worse
once you think you need more than a double pointer, you should rethink your data: use some structure to represent the matrix.

A simple example:

#include <stdlib.h>

struct matrix {
        unsigned nrow;
        unsigned ncol;
        // int flags;
        double *data;
        };

/*****************************************************************/
static size_t nrc2idx(unsigned ncol, unsigned irow, unsigned icol)
{
return (irow*ncol) + icol;
}

struct matrix *matrix_new(unsigned nrow, unsigned ncol)
{
struct matrix *mp;

mp = malloc (sizeof *mp);
if (!mp) return mp;
mp->data = malloc (sizeof *mp->data * nrow * ncol);
if ( !mp->data) {
        free (mp);
        return NULL;
        }
mp->nrow = nrow;
mp->ncol = ncol;

return mp;
}

Now, how hard is it to multiply two matrices, using this kind of struct? Example code:

struct matrix *matrix_mult(struct matrix *left, struct matrix *right)
{
struct matrix *result;
unsigned ii,jj;

if (!left || !right) return NULL;
if (left->ncol != right->nrow) return NULL;

result = matrix_new(left->nrow, right->ncol);
if (!result) return NULL;

for (ii=0; ii < result->nrow; ii++) {
        for (jj=0; jj < result->ncol; jj++) {
                size_t uu;
                unsigned kk;
                double sum ;
                sum = 0.0;
                for (kk=0; kk < left->ncol; kk++) {
                        size_t aa, bb;
                        aa = nrc2idx(left->ncol, ii, kk);
                        bb = nrc2idx(right->ncol, kk, jj);
                        sum += left->data[aa] * right->data[bb];
                        }
                uu = nrc2idx(result->ncol, ii, jj);
                result->data[uu] = sum;
                }
        }
return result;
}

it is suboptimal as well. Use proper types for sizes (`size_t`) not unsigned. Having too many returns is error prone (rather use positive checks instead negative) — 0___________, Jan 11 '22 at 14:49
If going down this route you should use flexible array members instead. Otherwise you haven't really solved the program with fragmented malloc calls and pointless levels of indirection. — Lundin, Jan 11 '22 at 14:50
I said it was a simplified example. It is only intended to show the concept of "encapsulation" (BTW: IMHO flexible array members are too hard for the OP, since he is still struggeling with pointers and arrays) — wildplasser, Jan 11 '22 at 14:56
I see the point you're making , However, am trying to reserve the `T[i][j]` format for my project , and also lack the skill to maintain integrity for this code when I implement it, as this OP is only a Library for SDL2_project. — Yacine Megrah, Jan 11 '22 at 15:45

Dynamic deallocation of a 2D matrix crashing

4 Answers4