valgrind memory lost in array of pointes

Question

I am using Valgrind to debug my c program. The error I receive is:

==2765== 8,000 bytes in 2 blocks are definitely lost in loss record 1 of 1
==2765==    at 0x4C274A8: malloc (vg_replace_malloc.c:236)
==2765==    by 0x404123: main (mycode.cpp:352)

Here is the code near line 352:

int **matrix;
matrix = (int**)malloc(2*sizeof(int*));
for (i=0; i<2; i++){
    matrix[i] = (int*)malloc(size*sizeof(int)); //line 352
}
for (i=0; i<2; i++){ //inizialization
    for (k=0; k<size; k++)
        matrix[i][k] = 0;
}

That is my way to allocate memory for a matrix. What is wrong with this?

Update: At the end of the program, I used:

free(matrix);

There's nothing wrong with the code you've posted. The error is in the code that frees `matrix`. Can you update your question to show this too please? — simonc, Feb 27 '13 at 10:20
I think I've said it before, but I'll say it again: [please don't cast the return value of `malloc()`, in C](http://stackoverflow.com/a/605858/28169). — unwind, Feb 27 '13 at 10:22
I updated the question as simonc asked. Thanks unwind :), I will read your post carefully. — altroware, Feb 28 '13 at 00:55

score 1 · Accepted Answer · answered Feb 27 '13 at 10:23

The valgrind output suggests that you are freeing matrix but not its members. You must have one call to free for each allocation:

for (i=0; i<2; i++) {
    free(matrix[i]);
}
free(matrix);

Note that you could simplify your code, avoiding the initialise to zero loops, if you allocated memory using calloc:

int **matrix = malloc(2*sizeof(int*));
for (i=0; i<2; i++){
    matrix[i] = calloc(size*sizeof(int));
}

Dariusz · Answer 2 · 2013-02-27T21:24:27.333

1

Why do you people all insist on allocating each row of an array separately? Just make one large alloc and a getter/setter method!

#define ARR_COLUMNS 10
#define ARR_ROWS 10

int* arr = calloc (ARR_COLUMNS * ARR_ROWS, sizeof(int));

int get(int* arr, int x, int y) {
  if (x<0 || x>= ARR_COLUMNS) return 0;
  if (y<0 || y>= ARR_ROWS) return 0;
  return arr[ARR_COLUMNS*y+x];
}

void set (int* arr, int x, int y, int val) {
  if (x<0 || x>= ARR_COLUMNS) return;
  if (y<0 || y>= ARR_ROWS) return;
  arr[ARR_COLUMNS*y+x] = val;
}

By doing so you will:

save yourself costly allocs and frees
have less fragmented memory
simplify your possible realloc calls
ensure the data is cached better and accessed without the common [x][y] vs [y][x] iteration cache problem.

edited Feb 27 '13 at 21:24

answered Feb 27 '13 at 10:24

Dariusz

21,561
9
74
114

Great! But I was wondering if it works also if I have to increase both the number of rows and column. – altroware Feb 28 '13 at 01:00
For 2D array reallocation you would have to write your own function that would `memmove` chunks of the array after realloc into their new places and zero the "new" part of the array. It is fairly simple, though must be done with care. – Dariusz Feb 28 '13 at 07:28

score 1 · Answer 3 · answered Feb 27 '13 at 12:11

Like simonc says, it sounds like you're not freeing the individual array elements.

If you're using a C99 compiler or a C2011 compiler that supports variable-length arrays, you can simplify things a bit and use a single malloc and free call like so:

int size;
...
int (*matrix)[size] = malloc(2 * sizeof *matrix);
...
// do stuff with matrix[i][j]
...
free (matrix);

If you're using a compiler that doesn't support VLAs, you'll either have to do the two-step allocation and deallocation, or allocate a 1-d array and map indices as in Darius' answer.

valgrind memory lost in array of pointes

3 Answers3