Little help with free()

[masterloki]

Hi there.

It's been a time since I did something in *real* C. Now I'm working in
something with pointers, but it seems that I can't get it right.

I have a int ** that I allocate with malloc, but when trying to free it
I get an error.

int initMatrix(int size, int **matrix){
int i=0;int j=0;
matrix = malloc(sizeof(int*)*size);
for (i=0;i<size+1;i++){
matrix= malloc(3 * sizeof(int));
}
for (i=0;i<size;i++)
for(j=0;j<2;j++){
matrix[j]=j;
printf("%d",matrix[j]);
}
for (i=0;i<size+1;i++){
free(matrix);
}
free(matrix);
}

I got this

*** glibc detected *** ./approach: double free or corruption (out):
0x00000000085b0030 ***
======= Backtrace: =========
/lib/libc.so.6[0x7f63d9174db6]
/lib/libc.so.6(cfree+0x6c)[0x7f63d91796fc]
../approach[0x804850d]
======= Memory map: ========
08048000-08049000 r-xp 00000000 08:06 131795
/home/masterloki/approach
08049000-0804a000 rwxp 00000000 08:06 131795
/home/masterloki/approach
085b0000-085d1000 rwxp 00000000 00:00 0
[heap]
7f63d4000000-7f63d4021000 rwxp 00000000 00:00 0
7f63d4021000-7f63d8000000 ---p 00000000 00:00 0
7f63d8eee000-7f63d8f04000 r-xp 00000000 08:06 7029
/usr/lib/libgcc_s.so.1
7f63d8f04000-7f63d9103000 ---p 00016000 08:06 7029
/usr/lib/libgcc_s.so.1
7f63d9103000-7f63d9104000 rwxp 00015000 08:06 7029
/usr/lib/libgcc_s.so.1
7f63d9104000-7f63d924d000 r-xp 00000000 08:06 700
/lib/libc-2.10.1.so
7f63d924d000-7f63d944d000 ---p 00149000 08:06 700
/lib/libc-2.10.1.so
7f63d944d000-7f63d9451000 r-xp 00149000 08:06 700
/lib/libc-2.10.1.so
7f63d9451000-7f63d9452000 rwxp 0014d000 08:06 700
/lib/libc-2.10.1.so
7f63d9452000-7f63d9457000 rwxp 00000000 00:00 0
7f63d9457000-7f63d9474000 r-xp 00000000 08:06 705
/lib/ld-2.10.1.so
7f63d964b000-7f63d964d000 rwxp 00000000 00:00 0
7f63d9671000-7f63d9673000 rwxp 00000000 00:00 0
7f63d9673000-7f63d9674000 r-xp 0001c000 08:06 705
/lib/ld-2.10.1.so
7f63d9674000-7f63d9675000 rwxp 0001d000 08:06 705
/lib/ld-2.10.1.so
7ffffc8e5000-7ffffc8fa000 rwxp 00000000 00:00 0
[stack]
7ffffc9ff000-7ffffca00000 r-xp 00000000 00:00 0
[vdso]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0
[vsyscall]
010101zsh: abort ./approach

I've compiled this with both gcc and tcc and I got the same error. I
debugged this with gdb and it breaks when I reach the free().
What am I doing wrong?

 

[Seebs]

int initMatrix(int size, int **matrix){
int i=0;int j=0;
matrix = malloc(sizeof(int*)*size);

You have allocated enough space for "size" pointer-to-int objects.

for (i=0;i<size+1;i++){
matrix= malloc(3 * sizeof(int));
}

You have initialized "size+1" pointer-to-int objects.

(Try working it out for size=1; you allocate one object, matrix[0], then
shove values in matrix[0] and matrix[1].)

for (i=0;i<size;i++)
for(j=0;j<2;j++){
matrix[j]=j;
printf("%d",matrix[j]);
}

And here, you fill matrix[0] and matrix[1], but not matrix[2].

In short, you have fencepost errors (so named because, with a fence post
every ten feet, you have 11 fence posts for 100 feet of fence).

The usual C idiom to avoid this:

N items are numbered 0..N-1. Always loop with:
for (i = 0; i < N; ++i)

If you find yourself doing "i < N+1" or "i <= N", you have probably got a
fencepost error, or will later when you come back to the code, not remembering
that you did this.

-s

 

[Barry Schwarz]

Hi there.

It's been a time since I did something in *real* C. Now I'm working in
something with pointers, but it seems that I can't get it right.

I have a int ** that I allocate with malloc, but when trying to free it
I get an error.

int initMatrix(int size, int **matrix){
int i=0;int j=0;
matrix = malloc(sizeof(int*)*size);

You allocate space for size pointers. Common usage numbers these 0
through size-1.

for (i=0;i<size+1;i++){
matrix= malloc(3 * sizeof(int));

You attempt to allocated space for 3 int in each "row".

In the last iteration, when i == size, you invoke undefined behavior
by attempting to store data beyond the end of your allocated area.

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

For some reason, you only initialize two of the three int in each row.

printf("%d",matrix[j]);
}
for (i=0;i<size+1;i++){
free(matrix);

The same undefined behavior in the last iteration.

}
free(matrix);
}

I got this

*** glibc detected *** ./approach: double free or corruption (out):
0x00000000085b0030 ***

It is a common manifestation that storing data beyond the allocation
boundaries messes up the internal control data that free needs.

I've compiled this with both gcc and tcc and I got the same error. I
debugged this with gdb and it breaks when I reach the free().
What am I doing wrong?

You are attempting to use more space than you allocate.

 

[Ben Bacarisse]

I won't repeat the malloc size advice you've had but I'd like to point
out something else:

It's been a time since I did something in *real* C. Now I'm working in
something with pointers, but it seems that I can't get it right.

I have a int ** that I allocate with malloc, but when trying to free
it I get an error.

int initMatrix(int size, int **matrix){
int i=0;int j=0;
matrix = malloc(sizeof(int*)*size);

This pattern looks odd. You pass in matrix (the type does not matter)
and immediately you set it so some value of your choosing. Why pass
the parameter at all? You might as well have written:

int initMatrix(int size) {
int i = 0, j = 0;
int **matrix = malloc(sizeof(int *) * size);

since neither the rest of the function nor the function's caller can
tell the difference.

<snip>

 

[masterloki]

Ben Bacarisse escribió:

I won't repeat the malloc size advice you've had but I'd like to point
out something else:


This pattern looks odd. You pass in matrix (the type does not matter)
and immediately you set it so some value of your choosing. Why pass
the parameter at all?

This is just a test, I'm writing several functions and I want to test if
the matrix is working (i.e. space allocated, values on it), since I'm
going to be moving that info around through several functions.

 

[masterloki]

pete escribió:

Hi there.

It's been a time since I did something in *real* C. Now I'm working in
something with pointers, but it seems that I can't get it right.

I have a int ** that I allocate with malloc, but when trying to free
it I get an error.

int initMatrix(int size, int **matrix){
int i=0;int j=0;
matrix = malloc(sizeof(int*)*size);
for (i=0;i<size+1;i++){
matrix= malloc(3 * sizeof(int));
}
for (i=0;i<size;i++)
for(j=0;j<2;j++){
matrix[j]=j;
printf("%d",matrix[j]);
}
for (i=0;i<size+1;i++){
free(matrix);
}
free(matrix);
}

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

#define J 2

void initMatrix(int size)
{
int i = 0;
int j = 0;
int **matrix = malloc(sizeof *matrix * size);

if (matrix != NULL) {
for (i = 0; size > i; ++i) {
matrix= malloc(J * sizeof *matrix);
}
for (i = 0; size > i; ++i) {
for (j = 0; J > j; ++j){
if (matrix != NULL) {
matrix[j] = j;
printf("%d ", matrix[j]);
}
}
putchar('\n');
}
for (i = 0; size > i; ++i){
free(matrix);
}
}
free(matrix);
}

Thanks for the reply

Just a question. Why use sizeof(*ptr) instead of size of sizeof(type)?

 

[masterloki]

Barry Schwarz escribió:

Hi there.

It's been a time since I did something in *real* C. Now I'm working in
something with pointers, but it seems that I can't get it right.

I have a int ** that I allocate with malloc, but when trying to free it
I get an error.

int initMatrix(int size, int **matrix){
int i=0;int j=0;
matrix = malloc(sizeof(int*)*size);

You allocate space for size pointers. Common usage numbers these 0
through size-1.

for (i=0;i<size+1;i++){
matrix= malloc(3 * sizeof(int));

You attempt to allocated space for 3 int in each "row".

In the last iteration, when i == size, you invoke undefined behavior
by attempting to store data beyond the end of your allocated area.

Unholy blasphemy, now I see it, after reading several times your reply I
see it. Pretty lame inned. That size+1 was because I thought I was
allocating more space to prevent this, but I didn't apply this when
allocating the first matrix part

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

For some reason, you only initialize two of the three int in each row.

printf("%d",matrix[j]);
}
for (i=0;i<size+1;i++){
free(matrix);

The same undefined behavior in the last iteration.

}
free(matrix);
}

I got this

*** glibc detected *** ./approach: double free or corruption (out):
0x00000000085b0030 ***

It is a common manifestation that storing data beyond the allocation
boundaries messes up the internal control data that free needs.

I've compiled this with both gcc and tcc and I got the same error. I
debugged this with gdb and it breaks when I reach the free().
What am I doing wrong?

You are attempting to use more space than you allocate.

Thanks for your reply and your help.

 

[James Kuyper]

Ben Bacarisse escribi�:

This is just a test, I'm writing several functions and I want to test if
the matrix is working (i.e. space allocated, values on it), since I'm
going to be moving that info around through several functions.

I think you've missed the point. The design of this function is
defective, and not in a way that's reasonably attributable to the fact
that you're just prototyping it. At the moment, this array you're
initializing is used only in this function, and it's free()'d before
this function exits. If that's going to be a permanent feature of this
program, then 'matrix' should be a block-scope variable, not a function
parameter.

If, on the other hand, you're planning to rewrite some future version of
this program so that initMatrix() initializes a matrix that some other
part of the program can then make use of, then 'matrix' needs to have at
least one additional level of indirection. As written, the calling
routine will never have any information that will allow it to make any
use of the memory that this function allocates.

 

[masterloki]

Seebs escribió:

int initMatrix(int size, int **matrix){
int i=0;int j=0;
matrix = malloc(sizeof(int*)*size);

You have allocated enough space for "size" pointer-to-int objects.

for (i=0;i<size+1;i++){
matrix= malloc(3 * sizeof(int));
}

You have initialized "size+1" pointer-to-int objects.

(Try working it out for size=1; you allocate one object, matrix[0], then
shove values in matrix[0] and matrix[1].)

for (i=0;i<size;i++)
for(j=0;j<2;j++){
matrix[j]=j;
printf("%d",matrix[j]);
}

And here, you fill matrix[0] and matrix[1], but not matrix[2].

In short, you have fencepost errors (so named because, with a fence post
every ten feet, you have 11 fence posts for 100 feet of fence).

The usual C idiom to avoid this:

N items are numbered 0..N-1. Always loop with:
for (i = 0; i < N; ++i)

If you find yourself doing "i < N+1" or "i <= N", you have probably got a
fencepost error, or will later when you come back to the code, not remembering
that you did this.

This is what actually happened. I the end, I forgot I wrote that.

-s

Thanks for yout reply. It was very useful!. After reading all responses,
I finally got it.

 

[masterloki]

James Kuyper escribió:

pete escribi�: ...

matrix= malloc(J * sizeof *matrix);

...
Just a question. Why use sizeof(*ptr) instead of size of sizeof(type)?

If, at some time in the future, you decide to change the type of ptr,
sizeof(type) will cease to be the correct size, but sizeof(*ptr) would
still be the correct size. More importantly, it's manifestly the correct
size; all that someone has to do is look at that one line to determine
that it's the correct size; with sizeof(type), you would have to first
look for the declaration of 'ptr' to determine whether it's correct.

Good practice, thanks for the advice

 

[masterloki]

James Kuyper escribió:

I think you've missed the point. The design of this function is
defective, and not in a way that's reasonably attributable to the fact
that you're just prototyping it. At the moment, this array you're
initializing is used only in this function, and it's free()'d before
this function exits. If that's going to be a permanent feature of this
program, then 'matrix' should be a block-scope variable, not a function
parameter.

If, on the other hand, you're planning to rewrite some future version of
this program so that initMatrix() initializes a matrix that some other
part of the program can then make use of, then 'matrix' needs to have at
least one additional level of indirection. As written, the calling
routine will never have any information that will allow it to make any
use of the memory that this function allocates.

This is function is going to be rewritten since it was for testing
purposes but even for testing I don't think it's a good idea to allocate
and don't free.

 

[James Kuyper]

James Kuyper escribió:

This is function is going to be rewritten since it was for testing
purposes but even for testing I don't think it's a good idea to allocate
and don't free.

The question is not whether you should free the memory, but where. It's
entirely appropriate, in some contexts, to define one function that
allocates the memory for a complicated data structure, and a separate
function that deallocates it. Those two functions should be treated as
bookend functions, just like malloc()/free() or fopen()/fclose(). A
function with the name "initMatrix" sounds like a good candidate for an
allocation function.

If that's not what it's intended to do - if all operations on the
allocated matrix will occur either in initMatrix or in subroutines
thereof - then "initMatrix" is a bad name for the function. In that
case, the matrix is purely an internal detail of no relevance to the
caller of the function. If that's true, then the name of the function
should be related to the purpose the function uses the matrix to
achieve, not the method it uses to achieve it.

 

[Keith Thompson]

Just a question. Why use sizeof(*ptr) instead of size of sizeof(type)?

Consider:

ptr = malloc(sizeof(type));

This is correct if an only if ptr is of type type*.

But this:

ptr = malloc(sizeof *ptr);

is correct as long as ptr is a pointer to *something* (well, to some
object type). Even if you change the definition of ptr later on, you
don't have to change the malloc call.