3

Working on some C code working with Matrices. I have created a Matrix struct to make it easier to create many instances of it quickly and efficiently but I am not sure why my code is leaking memory. Currently I have the following code:

    typedef struct
    {
    size_t rows;
    size_t cols;
    double ** value;
            }
    *Matrix;


Matrix matCreate( size_t rows, size_t cols )
{
        if(rows<=0 || cols<=0)
        {
                return NULL;
        }
        Matrix mat = (Matrix) malloc(sizeof(Matrix));
        mat->rows = rows;
        mat->cols = cols;
        mat->value = (double **) malloc(rows*sizeof(double*));
        for (int i=0; i< rows;i++)
        {
                mat->value[i] = (double *) malloc(cols * sizeof(double));
                for( int j=0; j< cols;j++)
                {
                        mat->value[i][j] = 0;

                        if(rows == cols && i==j )
                        {
                                mat->value[i][j] = 1;
                        }
                }

        }
        return mat;
}

And I am getting the following memory leaks after running valgrind. When running the code it compiles completely without error and still outputs the right output.

==23609== Invalid write of size 8
==23609==    at 0x400800: matCreate 
==23609==    by 0x4010E2: main 
==23609==  Address 0x5203048 is 0 bytes after a block of size 8 alloc'd
==23609==    at 0x4C2DB8F: malloc 
==23609==    by 0x4007E8: matCreate 
==23609==    by 0x4010E2: main
==23609==
==23609== Invalid write of size 8
==23609==    at 0x40081B: matCreate 
==23609==    by 0x4010E2: main 
==23609==  Address 0x5203050 is 8 bytes after a block of size 8 alloc'd
==23609==    at 0x4C2DB8F: 
==23609==    by 0x4007E8: matCreate
==23609==    by 0x4010E2: main
==23609==
==23609== Invalid read of size 8
==23609==    at 0x40082F: matCreate 
==23609==    by 0x4010E2: main 
==23609==  Address 0x5203050 is 8 bytes after a block of size 8 alloc'd
==23609==    at 0x4C2DB8F: malloc 
==23609==    by 0x4007E8: matCreate 
==23609==    by 0x4010E2: main
WasabiCannon
  • 139
  • 2
  • 10
  • Try compiling with `-g` so `valgrind` will show you the exact line on which those accesses occur... – BadZen Nov 11 '16 at 23:24
  • There is no Matrix (aka 2D array) in your code. A pointer is not an array! If you need matrices use a 2D array. And don't cast the result of `malloc` & friends or `void *` in general. – too honest for this site Nov 12 '16 at 00:27
  • Learn the following memory allocation idiom: `T p = malloc(sizeof *p)` or `T p = malloc(N * sizeof *p)`, where `T` is some pointer type. I.e. no typenames under `sizeof` and no casting the result of `malloc`. This will save you from such blunders in the futrure. – AnT stands with Russia Nov 12 '16 at 00:48

1 Answers1

3

The line

    Matrix mat = (Matrix) malloc(sizeof(Matrix));

is not good. It does not allocate enough memory. As a consequence, your program has undefined behavior.

size(Memory) evaluates to the size of a pointer, not the size of the struct.

It needs to be:

    Matrix mat = malloc(sizeof(*mat));

Defining Matrix that is really a pointer is not good coding practice. It will lead to confusion. I suggest defining it as:

typedef struct
{
   size_t rows;
   size_t cols;
   double ** value;
} Matrix;

and then use:

Matrix* matCreate( size_t rows, size_t cols ) { ... }

...

Matrix* mat = malloc(sizeof(*mat));

See Do I cast the result of malloc?

Community
  • 1
  • 1
R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • When changing my code to " Matrix mat = (Matrix) malloc(sizeof(*Matrix))" I am getting error: expected expression before ‘Matrix’ Matrix mat = (Matrix) malloc(sizeof(*Matrix)); – WasabiCannon Nov 11 '16 at 23:33
  • 1
    `sizeof(*Matrix)` is invalid. `Matrix` is a type. You cannot "deference" a type in C. You cannot transform pointer type to pontee type by applying `*` operator to it. There's no such feature in C, however logical it might look at the first sight. – AnT stands with Russia Nov 12 '16 at 00:37
  • The second part of your answer also suffers from typos. Since you redefined `Matrix` as struct type (instead of the original pointer type), the cast you apply to `malloc` result should be `(Matrix *)`, not `(Matrix)`. Of course, a better idea would be not to cast at all. – AnT stands with Russia Nov 12 '16 at 00:45